Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add --fail-on-warning option #1093

Merged
merged 1 commit into from
Nov 18, 2017
Merged

Add --fail-on-warning option #1093

merged 1 commit into from
Nov 18, 2017

Conversation

yuku
Copy link
Contributor

@yuku yuku commented May 17, 2017 •
edited
Loading

Description

Add --fail-on-warning option. See #935

Completed Tasks

  • I have read the Contributing Guide.
  • The pull request is complete (implemented / written).
  • Git commits have been cleaned up (squash WIP / revert commits).
  • I wrote tests and ran bundle exec rake locally (if code is attached to PR).

@coveralls
Copy link

coveralls commented May 17, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.008%) to 93.471% when pulling 8ea5b52 on yuku-t:fail-on-warning into f16cf4c on lsegal:master.

@coveralls
Copy link

coveralls commented May 17, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.008%) to 93.47% when pulling 8438e9d on yuku-t:fail-on-warning into f16cf4c on lsegal:master.

@yuku
Copy link
Contributor Author

yuku commented May 17, 2017 •
edited
Loading

Fixed typo and pushed forcibly

olleolleolle
olleolleolle reviewed May 17, 2017
View reviewed changes
lib/yard/cli/yardoc.rb Outdated
@@ -200,6 +200,9 @@ class Yardoc < YardoptsCommand
# @since 0.7.0
attr_accessor :has_markup

# @return [Boolean] whether yard exits with error status code if an warning occurs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: if an warning => if a warning

olleolleolle
olleolleolle reviewed May 17, 2017
View reviewed changes
lib/yard/cli/yardoc.rb Outdated
@@ -273,6 +277,8 @@ def run(*args)
end
end

exit 1 if fail_on_warning && log.warned
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use abort instead of exit 1 - the (slight) benefit abort has is that the provided failing exit status code is something which works for the current platform.

olleolleolle
olleolleolle reviewed May 17, 2017
View reviewed changes
lib/yard/cli/yardoc.rb Outdated
@@ -555,6 +561,10 @@ def general_options(opts)
opts.on('--exclude REGEXP', 'Ignores a file if it matches path match (regexp)') do |path|
excluded << path
end

opts.on('--fail-on-warning', 'Exit with error status code if an warning occurs') do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo an warning => a warning

olleolleolle
olleolleolle reviewed May 17, 2017
View reviewed changes
lib/yard/logging.rb Outdated
@@ -60,6 +61,13 @@ def debug(*args)
super
end

# Remember an warning occurs and writes a warning message.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps: Remembers when a warning ...

olleolleolle
olleolleolle reviewed May 17, 2017
View reviewed changes
Copy link
Contributor

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a nice feature!

@coveralls
Copy link

coveralls commented May 17, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.008%) to 93.47% when pulling 92b9b7c on yuku-t:fail-on-warning into f16cf4c on lsegal:master.

@yuku
Copy link
Contributor Author

yuku commented May 17, 2017

@olleolleolle Thanks for reviewing 👍
I've updated the patch.

@coveralls
Copy link

coveralls commented May 17, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.008%) to 93.47% when pulling 92b9b7c on yuku-t:fail-on-warning into f16cf4c on lsegal:master.

@yoshiwaan
Copy link

Any reason this hasn't been merged? If so is there anything I can do help it along?

I'd like to have yard validation a part of our PR review checks

@olleolleolle
Copy link
Contributor

@yoshiwaan Would this project serve your needs for that? https://github.com/zverok/yard-junk

@lsegal lsegal merged commit fe7145c into lsegal:master Nov 18, 2017
lsegal added a commit that referenced this pull request Nov 18, 2017
@lsegal
Tag release v0.9.10
9fcfe09 
References:
  #1023, #1077, #1079, #1089, #1093, #1097, #1099, #1101,
  #1102, #1104, #1111, #1119, #1123, #1124, #1126, #1128,
  #1133, #1135
@yuku yuku deleted the fail-on-warning branch November 19, 2017 12:12
@kachick kachick mentioned this pull request Apr 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olleolleolle olleolleolle olleolleolle left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yuku @coveralls @yoshiwaan @olleolleolle @lsegal