Skip to content

Check that --from is actually provided #73

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.

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

Merged
merged 1 commit into from
May 27, 2018

Conversation

ciaranmcnulty
Copy link
Contributor

@ciaranmcnulty ciaranmcnulty commented May 27, 2018

This addresses #72 - in my instance hasOption is returning true because its been replaced with its default value of null.

I'm mildly surprised that this hasn't happened to anyone pre-release though - are you always specifying a revision? It's possible that this is a symfony version thing.

I stuck to the existing paradigm for the tests but there's a lot of stubbing going on - it might be clearer to use a concrete ArgvInput (which would also have avoided the issue of stubbing enshrining an incorrect understanding of the API)

@Ocramius Ocramius self-assigned this May 27, 2018
@Ocramius Ocramius added the bug label May 27, 2018
@Ocramius Ocramius added this to the 1.0.2 milestone May 27, 2018
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

🚢

@Ocramius Ocramius merged commit cd87d1f into Roave:master May 27, 2018
@Ocramius Ocramius changed the title Check that from is actually provided Check that --from is actually provided May 27, 2018
@Ocramius
Copy link
Member

Hmm, no, this patch completely killed --from support, as a precise value is being expected here.

I'll probably have to revert it completely and apply #66 instead.

@ciaranmcnulty
Copy link
Contributor Author

Sound fair - but write some end-to-end tests

@Ocramius
Copy link
Member

Ocramius commented May 27, 2018 via email

Ocramius added a commit that referenced this pull request May 28, 2018
…OR* is picked

Previously, only the *MINOR* version was being compared, leading to previous major
versions being considered in the lookup for the lowest available stable release.
Ocramius added a commit that referenced this pull request May 28, 2018
…itories

The reasoning behind this is that `symfony/console` has a very squishy API, and
that can lead to a lot of headaches in upgrades or implicit API changes.

Having an E2E test will ensure that the implementation details of `symfony/console`
are also considered, since the interfaces really just lie.
Ocramius added a commit that referenced this pull request May 28, 2018
Ocramius added a commit that referenced this pull request May 28, 2018
Ocramius added a commit that referenced this pull request May 28, 2018
… comparisons on no valid detected versions
Ocramius added a commit that referenced this pull request May 28, 2018
…d in the selected stable minor releases
Ocramius added a commit that referenced this pull request May 28, 2018
Ocramius added a commit that referenced this pull request Jun 4, 2018
Ocramius added a commit that referenced this pull request Jun 4, 2018
Ocramius added a commit that referenced this pull request Jun 4, 2018
Ocramius added a commit that referenced this pull request Jun 4, 2018
…options

#66 #72 #73 fixed missing `--from` handling, minor version detection and scenario with no detected versions at all
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants