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

[BUGFIX] Fix no-docs input option #41

Closed
wants to merge 1 commit into from
Closed

[BUGFIX] Fix no-docs input option #41

wants to merge 1 commit into from

Conversation

gilbertsoft
Copy link
Contributor

Fixes #38

o-ba
o-ba reviewed Nov 17, 2021
View reviewed changes
Copy link
Member

@o-ba o-ba left a comment

Choose a reason for hiding this comment

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

Hi, besides the non existing constant, I thought about this again and fear this would be a breaking change for people, already using --no-docs=1 in their workflows, because this would then fail with:

The "--no-docs" option does not accept a value.

Additionally, the check in L:76 would need to be adjusted since NULL is no longer to be expected.

src/Command/Extension/SetExtensionVersionCommand.php Outdated Show resolved Hide resolved
@gilbertsoft
Copy link
Contributor Author

Hi, besides the non existing constant, I thought about this again and fear this would be a breaking change for people, already using --no-docs=1 in their workflows, because this would then fail with:

The "--no-docs" option does not accept a value.

Additionally, the check in L:76 would need to be adjusted since NULL is no longer to be expected.

Agree, this is ofc kind of breaking. Will fix the error so it's ready to merge when the right time comes or feel free to close.

@o-ba
Copy link
Member

o-ba commented Nov 18, 2021

Agree, this is ofc kind of breaking. Will fix the error so it's ready to merge when the right time comes or feel free to close.

Oke I would like to postpone this for now. Thanks for the adjustments, we might bring this up again in the future.

@o-ba o-ba closed this Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@o-ba o-ba o-ba 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.

set-version no-docs option looks wrong
2 participants
@gilbertsoft @o-ba