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.

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

Propose semantic versioning of the library #18

Merged
merged 1 commit into from
Feb 5, 2018

Conversation

Ocramius
Copy link
Member

Since we released 2.0.0 of this library, we started adding more rules in 2.1.0.
This led to consumers of the library to lock onto ~2.1.0 rather than ^2.1.0.

Effectively, this means that our consumers (library authors) consider our minor
releases to be major releases (and rightfully so!).

This patch suggests to use major versions for any addition to the ruleset that may
lead to code change requirements downstream in consumers, and makes this package
"less special" among the dependencies of any project relying on it.

Since we released `2.0.0` of this library, we started adding more rules in `2.1.0`.
This led to consumers of the library to lock onto `~2.1.0` rather than `^2.1.0`.

Effectively, this means that our consumers (library authors) consider our minor
releases to be major releases (and rightfully so!).

This patch suggests to use major versions for any addition to the ruleset that may
lead to code change requirements downstream in consumers, and makes this package
"less special" among the dependencies of any project relying on it.
@Ocramius
Copy link
Member Author

Also, can we get @carusogabriel on board on this project? This seems to be his area of interest, so I think he's a better fit to closely follow improvements in the CS ruleset :-)

@alcaeus
Copy link
Member

alcaeus commented Jan 29, 2018

Makes sense - @Majkl578 and I discussed this a while back and decided to follow the path Slevomat themselves go. Difference is: they suggest not including the entire ruleset but rather selectively importing them, while we suggest to import the entire ruleset.

I'm okay with the change, we'll just bump major releases more often then 👍

Sneak edit: I'm also 👍 with bringing @carusogabriel in.

@carusogabriel
Copy link
Contributor

@Ocramius Thank you for inviting me. I'm still learning and study Coding Standards, Tokens, all this stuff, but nothing that some nights awake don't solve 😁

I still need to propose the Null Coalesce Operator and New with parentheses to a PHPCS library, so we can add here.

Also, the Simplify returns I don't think we can have a sniff for that, as is something that doesn't have a standard way to verify with Sniffs, is something more visual 😞

@Ocramius
Copy link
Member Author

Ocramius commented Jan 29, 2018

@carusogabriel please open different PRs (can be just WIP README.md changes for now) for work on these 3. We should also split long conditional expressions that return bool into multiple return statements ;-)

@carusogabriel
Copy link
Contributor

We should also split long conditional expressions that return bool into multiple return statements

Something like:

if ($foo === 'bar' || $foo === 'baz' || $foo !== 'foo') {
    return true;
}

into:

if ($foo === 'bar') {
    return true;
}

if ($foo === 'baz') {
    return true;
}

if ($foo !== 'foo') {
    return true;
}

?

@Ocramius
Copy link
Member Author

@carusogabriel open PR, let's discuss there.

@Majkl578
Copy link
Contributor

I'm ok with this once we settle down with the new rules (i.e. those @carusogabriel proposes). Original intention was to not blow versioning to 10+ just by adding some new sniffs, but once these new sniffs are on board (+ those that already are) it's ok to go back to semver. 👍

@Majkl578 Majkl578 modified the milestones: 2.1.0, 3.0.0 Feb 5, 2018
Copy link
Contributor

@Majkl578 Majkl578 left a comment

Choose a reason for hiding this comment

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

🚢
I'll update constraint & alias separately.

@Majkl578 Majkl578 merged commit 5827927 into master Feb 5, 2018
@Majkl578 Majkl578 deleted the propose-semantic-versioning branch February 5, 2018 22:55
@Ocramius
Copy link
Member Author

Ocramius commented Feb 6, 2018

Btw, I included @carusogabriel in the organisation, with write access to this repo. /cc @doctrine/doctrinecore

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

Successfully merging this pull request may close these issues.

None yet

4 participants