Add --fail-on-warning option #1093

Merged
merged 1 commit into from Nov 18, 2017

Conversation

Projects
None yet
5 participants
@yuku
Contributor

yuku commented May 17, 2017

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

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 17, 2017

Coverage Status

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

coveralls commented May 17, 2017

Coverage Status

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

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 17, 2017

Coverage Status

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

coveralls commented May 17, 2017

Coverage Status

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

@yuku

This comment has been minimized.

Show comment
Hide comment
@yuku

yuku May 17, 2017

Contributor

Fixed typo and pushed forcibly

Contributor

yuku commented May 17, 2017

Fixed typo and pushed forcibly

lib/yard/cli/yardoc.rb
@@ -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

This comment has been minimized.

@olleolleolle

olleolleolle May 17, 2017

Contributor

Typo: if an warning => if a warning

@olleolleolle

olleolleolle May 17, 2017

Contributor

Typo: if an warning => if a warning

lib/yard/cli/yardoc.rb
@@ -273,6 +277,8 @@ def run(*args)
end
end
+ exit 1 if fail_on_warning && log.warned

This comment has been minimized.

@olleolleolle

olleolleolle May 17, 2017

Contributor

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 May 17, 2017

Contributor

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.

lib/yard/cli/yardoc.rb
@@ -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

This comment has been minimized.

@olleolleolle

olleolleolle May 17, 2017

Contributor

Typo an warning => a warning

@olleolleolle

olleolleolle May 17, 2017

Contributor

Typo an warning => a warning

lib/yard/logging.rb
@@ -60,6 +61,13 @@ def debug(*args)
super
end
+ # Remember an warning occurs and writes a warning message.

This comment has been minimized.

@olleolleolle

olleolleolle May 17, 2017

Contributor

Perhaps: Remembers when a warning ...

@olleolleolle

olleolleolle May 17, 2017

Contributor

Perhaps: Remembers when a warning ...

@olleolleolle

I think this is a nice feature!

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 17, 2017

Coverage Status

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

coveralls commented May 17, 2017

Coverage Status

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

@yuku

This comment has been minimized.

Show comment
Hide comment
@yuku

yuku May 17, 2017

Contributor

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

Contributor

yuku commented May 17, 2017

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

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 17, 2017

Coverage Status

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

coveralls commented May 17, 2017

Coverage Status

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

@yoshiwaan

This comment has been minimized.

Show comment
Hide comment
@yoshiwaan

yoshiwaan Nov 13, 2017

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

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

This comment has been minimized.

Show comment
Hide comment
@olleolleolle

olleolleolle Nov 13, 2017

Contributor

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

Contributor

olleolleolle commented Nov 13, 2017

@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

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

lsegal added a commit that referenced this pull request Nov 18, 2017

@yuku yuku deleted the yuku:fail-on-warning branch Nov 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment