-
-
Notifications
You must be signed in to change notification settings - Fork 73
4.0 | Rename master branch to stable #3
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
Comments
About the renaming, I think it's a good idea, about the default branch… I'm not sure. In |
@greg0ire Thanks for your input. I'm going to have to have a think about that.
If I understand you correctly, you mean that My intention is to use The one thing I really don't want is to have long running "next major" branches, as is currently the case with the four year running |
Yes exactly I misunderstood what
Ok so, if I understand correctly, this workflow is going to lead in a situation more similar to what firefox does, where there are major releases very often?
That looks a lot with what I had to do with all the backports from 3 to 2. I feel your pain. |
I'd like to know how you handle bug fixes, new features, and BC breaks in PRs using a single branch. In cases where the |
@greg0ire It's akin to a git-flow workflow, but not as strict ;-)
Well, hopefully not with that much version inflation, but yes, I do intend to allow for more regular majors when the situation calls for it. I have a rough outline of a roadmap/plan for 4.0 -> 5.0 -> 6.0 already, but the first focus in that regards is getting 4.0 in a releasable state, so I don't want to share too many ideas for the future at this time for fear of overpromising.
@asispts Well, delays like that may sometimes happen, but I would mostly try to avoid them by planning out merges carefully, similar to what has been done in this repo in the past. If you look at the open PR list now, there are some things milestoned for When getting close to the next major, I would create a 3.x branch for the final patch/minor releases of the old major and then let And yes, even though I could push straight to the
Does that help clarify things a bit more ? |
For me it does 🙂 If I understand correctly, you're "storing" improvements in the next minor milestone, and breaking changes in the next major milestone, and then when the time comes, you merge all at once, probably not with an octopus merge though 😅 I guess you don't wait for too long though because otherwise the merge/rebases must be quite painful. It sounds like a lot of work to be honest, but I suppose it all depends on the actual number of PRs in a milestone, and from what I saw in 3.9.0 there aren't that many (and since you're the author of many of them, and the PRs are recent, it's probably not that hard). 100% agree on using PRs for absolutely every change 👍 |
@greg0ire Correct and yes, I don't intend to let things wait for too long. I also have a GH Actions workflow already set up which automatically labels PRs with merge conflicts, so it's pretty straight forward to see what needs a rebase and if there is one thing I've become good at over the years, it's rebases, so I'm not concerned about the current merge flow and will be happy to revisit if it would become a problem. |
@greg0ire Just read up on octopus merges, hadn't come across that term before. And no, when I do those kind of "merge sprees", I will merge one PR at a time and for all my own PRs, rebase before merging the next one, to make sure that if there is a CI failure due to one change having an unexpected impact elsewhere, it can be easily identified and gets caught before the merge. That's also how I merged my own PRs which had been waiting to be merged in the squizlabs repo (for ages) and which I'd ported over and included in the 3.8.0 release. |
I've done them once in my life, at work, for a merge release train, in a case when I was somehow sure in advance there wouldn't be any conflicts. It was fun but I wouldn't recommend it 🤣 The way you're describing is safer indeed 👍 |
This adds another set of dedicated tests to safeguard how XML docs which don't follow the specification are handled. This initial set of tests for this documents the current behaviour [*]. This behaviour may not always be the desired behaviour, in which case, this will be fixed in follow-up commits. To get these tests up and running, the following fatal errors needed to be fixed: * Fatal error when a code comparison only contains a single code element: ``` Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:253 Stack trace: #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement)) #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement)) #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate() #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS() #4 {main} thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 253 ``` * Fatal error when a code element contains no textual content: ``` Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:246 Stack trace: #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement)) #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement)) #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate() #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS() #4 {main} thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 246 ``` Both of these fatals are fixed by adding defensive coding validating that there are (at least) two code blocks to the `getFormattedCodeComparisonBlock()` methods for all three generator classes.
This adds another set of dedicated tests to safeguard how XML docs which don't follow the specification are handled. This initial set of tests for this documents the current behaviour [*]. This behaviour may not always be the desired behaviour, in which case, this will be fixed in follow-up commits. To get these tests up and running, the following fatal errors needed to be fixed: * Fatal error when a code comparison only contains a single code element: ``` Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:253 Stack trace: #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement)) #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement)) #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate() #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS() #4 {main} thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 253 ``` * Fatal error when a code element contains no textual content: ``` Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:246 Stack trace: #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement)) #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement)) #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate() #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS() #4 {main} thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 246 ``` Both of these fatals are fixed by adding defensive coding validating that there are (at least) two code blocks to the `getFormattedCodeComparisonBlock()` methods for all three generator classes.
This adds another set of dedicated tests to safeguard how XML docs which don't follow the specification are handled. This initial set of tests for this documents the current behaviour [*]. This behaviour may not always be the desired behaviour, in which case, this will be fixed in follow-up commits. To get these tests up and running, the following fatal errors needed to be fixed: * Fatal error when a code comparison only contains a single code element: ``` Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:253 Stack trace: #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement)) #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement)) #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate() #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS() #4 {main} thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 253 ``` * Fatal error when a code element contains no textual content: ``` Fatal error: Uncaught Error: Call to a member function getAttribute() on null in path/to/PHP_CodeSniffer/src/Generators/Markdown.php:246 Stack trace: #0 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(139): PHP_CodeSniffer\Generators\Markdown->getFormattedCodeComparisonBlock(Object(DOMElement)) #1 path/to/PHP_CodeSniffer/src/Generators/Markdown.php(39): PHP_CodeSniffer\Generators\Markdown->processSniff(Object(DOMElement)) #2 path/to/PHP_CodeSniffer/src/Runner.php(99): PHP_CodeSniffer\Generators\Markdown->generate() #3 path/to/PHP_CodeSniffer/bin/phpcs(14): PHP_CodeSniffer\Runner->runPHPCS() #4 {main} thrown in path/to/PHP_CodeSniffer/src/Generators/Markdown.php on line 246 ``` Both of these fatals are fixed by adding defensive coding validating that there are (at least) two code blocks to the `getFormattedCodeComparisonBlock()` methods for all three generator classes.
This fixes the following fatal error which would occur if the `Diff` report is used in combination with caching: ``` Fatal error: Uncaught Error: Call to a member function getFixableCount() on null in path/to/PHP_CodeSniffer/src/Fixer.php:144 Stack trace: #0 path/to/PHP_CodeSniffer/src/Reports/Diff.php(73): PHP_CodeSniffer\Fixer->fixFile() #1 path/to/PHP_CodeSniffer/src/Reporter.php(285): PHP_CodeSniffer\Reports\Diff->generateFileReport(Array, Object(PHP_CodeSniffer\Files\LocalFile), true, 150) #2 path/to/PHP_CodeSniffer/src/Runner.php(659): PHP_CodeSniffer\Reporter->cacheFileReport(Object(PHP_CodeSniffer\Files\LocalFile)) #3 path/to/PHP_CodeSniffer/src/Runner.php(400): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile)) #4 path/to/PHP_CodeSniffer/src/Runner.php(119): PHP_CodeSniffer\Runner->run() #5 path/to/PHP_CodeSniffer/bin/phpcs(30): PHP_CodeSniffer\Runner->runPHPCS() #6 {main} thrown in path/to/PHP_CodeSniffer/src/Fixer.php on line 144 ```
…report Okay, so this is an awkward one. If both the `Code` + the `Diff` report were requested + caching was turned on + the _order_ of the requested reports was `Code,Diff` (i.e. first the code report), the following fatal error which would occur: ``` Fatal error: Uncaught Error: Call to a member function getFixableCount() on null in path/to/PHP_CodeSniffer/src/Fixer.php:144 Stack trace: #0 path/to/PHP_CodeSniffer/src/Reports/Diff.php(73): PHP_CodeSniffer\Fixer->fixFile() #1 path/to/PHP_CodeSniffer/src/Reporter.php(285): PHP_CodeSniffer\Reports\Diff->generateFileReport(Array, Object(PHP_CodeSniffer\Files\LocalFile), true, 150) #2 path/to/PHP_CodeSniffer/src/Runner.php(659): PHP_CodeSniffer\Reporter->cacheFileReport(Object(PHP_CodeSniffer\Files\LocalFile)) #3 path/to/PHP_CodeSniffer/src/Runner.php(400): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile)) #4 path/to/PHP_CodeSniffer/src/Runner.php(119): PHP_CodeSniffer\Runner->run() #5 path/to/PHP_CodeSniffer/bin/phpcs(30): PHP_CodeSniffer\Runner->runPHPCS() #6 {main} thrown in path/to/PHP_CodeSniffer/src/Fixer.php on line 144 ``` To reproduce the issue (on `master`): 1. Create a small test file like: ```php <?php echo 'hello'.callMe( $p ); ``` 2. Run the following command to initialize the cache: ```bash phpcs -ps ./test.php --standard=PSR2 --report=Code,Diff --cache ``` All should be fine. 3. Now run the same command again and see the fatal error. Some investigating later, it turns out that both the `Code` report, as well as the `Diff` report, re-parse the current file, with the `Diff` report initializing the fixer once the file has reparsed, but the `Code` report not doing so (as it doesn't need the fixer). The problem with that is that the `Code` report basically leaves the `Fixer` in an invalid state, leading to the above error. While it is up for debate whether reports should ever (be allowed to) re-parse files, for now, let's fix the fatal. Re-evaluating the report setup should definitely be done, but should probably wait until the Reports classes have test coverage.
…report Okay, so this is an awkward one. If both the `Code` + the `Diff` report were requested + caching was turned on + the _order_ of the requested reports was `Code,Diff` (i.e. first the code report), the following fatal error which would occur: ``` Fatal error: Uncaught Error: Call to a member function getFixableCount() on null in path/to/PHP_CodeSniffer/src/Fixer.php:144 Stack trace: #0 path/to/PHP_CodeSniffer/src/Reports/Diff.php(73): PHP_CodeSniffer\Fixer->fixFile() #1 path/to/PHP_CodeSniffer/src/Reporter.php(285): PHP_CodeSniffer\Reports\Diff->generateFileReport(Array, Object(PHP_CodeSniffer\Files\LocalFile), true, 150) #2 path/to/PHP_CodeSniffer/src/Runner.php(659): PHP_CodeSniffer\Reporter->cacheFileReport(Object(PHP_CodeSniffer\Files\LocalFile)) #3 path/to/PHP_CodeSniffer/src/Runner.php(400): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile)) #4 path/to/PHP_CodeSniffer/src/Runner.php(119): PHP_CodeSniffer\Runner->run() #5 path/to/PHP_CodeSniffer/bin/phpcs(30): PHP_CodeSniffer\Runner->runPHPCS() #6 {main} thrown in path/to/PHP_CodeSniffer/src/Fixer.php on line 144 ``` To reproduce the issue (on `master`): 1. Create a small test file like: ```php <?php echo 'hello'.callMe( $p ); ``` 2. Run the following command to initialize the cache: ```bash phpcs -ps ./test.php --standard=PSR2 --report=Code,Diff --cache ``` All should be fine. 3. Now run the same command again and see the fatal error. Some investigating later, it turns out that both the `Code` report, as well as the `Diff` report, re-parse the current file, with the `Diff` report initializing the fixer once the file has reparsed, but the `Code` report not doing so (as it doesn't need the fixer). The problem with that is that the `Code` report basically leaves the `Fixer` in an invalid state, leading to the above error. While it is up for debate whether reports should ever (be allowed to) re-parse files, for now, let's fix the fatal. Re-evaluating the report setup should definitely be done, but should probably wait until the Reports classes have test coverage.
... and add a
develop
branch as the default WIP branch. Thestable
branch will become the release-only branch.The text was updated successfully, but these errors were encountered: