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

Enhancement: Implement VersionConstraintNormalizer #18

Merged
merged 1 commit into from
Jan 19, 2018
Merged

Enhancement: Implement VersionConstraintNormalizer #18

merged 1 commit into from
Jan 19, 2018

Conversation

localheinz
Copy link
Member

@localheinz localheinz commented Jan 15, 2018 •
edited
Loading

This PR

  • implements a VersionConstraintNormalizer

Fixes #16.

@localheinz localheinz self-assigned this Jan 15, 2018
@localheinz localheinz force-pushed the feature/version-constraint-normalizer branch from 5b21027 to cd2bd9e Compare January 15, 2018 21:16
@codecov
Copy link

codecov bot commented Jan 15, 2018 •
edited
Loading

Codecov Report

Merging #18 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##             master    #18   +/-   ##
=======================================
  Coverage       100%   100%           
- Complexity       39     47    +8     
=======================================
  Files             6      7    +1     
  Lines           131    162   +31     
=======================================
+ Hits            131    162   +31
Impacted Files Coverage Δ Complexity Δ
src/Normalizer/ComposerJsonNormalizer.php 100% <100%> (ø) 4 <0> (ø) ⬇️
src/Normalizer/VersionConstraintNormalizer.php 100% <100%> (ø) 8 <8> (?)
src/Command/NormalizeCommand.php 100% <0%> (ø) 13% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5817eea...5bb833f. Read the comment docs.

@localheinz localheinz force-pushed the feature/version-constraint-normalizer branch from cd2bd9e to e45677e Compare January 15, 2018 21:57
@soullivaneuh
Copy link

For example, trimming between version constraints

Yeah!

ensuring a single space between them

Looks good, but maybe we should find official examples on getcomposer.org

@soullivaneuh
Copy link

I can't find official example, but I always see something like ^2.0,<=2.0.4 without any space. So it should be ok.

soullivaneuh
soullivaneuh suggested changes Jan 16, 2018
View reviewed changes
README.md Outdated
* `require`
* `require-dev`

sections, the `VersionOrConstraintNormalizer` will ensure that a `||` is
Copy link

Choose a reason for hiding this comment

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

|| (with spaces around)

@localheinz localheinz force-pushed the feature/version-constraint-normalizer branch from e45677e to e9c2bdd Compare January 18, 2018 22:33
@localheinz localheinz changed the title Enhancement: Implement VersionOrConstraintNormalizer Enhancement: Implement VersionConstraintNormalizer Jan 18, 2018
@localheinz
Copy link
Member Author

@soullivaneuh

Would you like to take another look?

@localheinz localheinz force-pushed the feature/version-constraint-normalizer branch 6 times, most recently from 45bafad to 5bb833f Compare January 19, 2018 22:23
@localheinz
Copy link
Member Author

@soullivaneuh

I incorporated all examples I could find at https://getcomposer.org/doc/articles/versions.md and think of, I think this is good to merge (once the build has passed).

@codecov-io
Copy link

codecov-io commented Jan 19, 2018 •
edited
Loading

Codecov Report

Merging #18 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##             master    #18   +/-   ##
=======================================
  Coverage       100%   100%           
- Complexity       39     48    +9     
=======================================
  Files             6      7    +1     
  Lines           131    163   +32     
=======================================
+ Hits            131    163   +32
Impacted Files Coverage Δ Complexity Δ
src/Normalizer/VersionConstraintNormalizer.php 100% <100%> (ø) 9 <9> (?)
src/Normalizer/ComposerJsonNormalizer.php 100% <100%> (ø) 4 <0> (ø) ⬇️
src/Command/NormalizeCommand.php 100% <0%> (ø) 13% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5817eea...5bb833f. Read the comment docs.

@localheinz localheinz force-pushed the feature/version-constraint-normalizer branch from 5bb833f to fcc5ca1 Compare January 19, 2018 22:37
@localheinz localheinz force-pushed the feature/version-constraint-normalizer branch from fcc5ca1 to 0eb0d22 Compare January 19, 2018 22:38
@localheinz localheinz merged commit d287f07 into master Jan 19, 2018
@localheinz localheinz deleted the feature/version-constraint-normalizer branch January 19, 2018 22:38
@localheinz localheinz mentioned this pull request Jan 19, 2018
Merged
1 task
@soullivaneuh
Copy link

Sorry for the delay, but it looks great!

@localheinz
Copy link
Member Author

Glad you like it, @soullivaneuh!

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

@soullivaneuh soullivaneuh soullivaneuh requested changes

Assignees

@localheinz localheinz

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@localheinz @soullivaneuh @codecov-io