VariableOrderWrong: Enforce skel.ebuild variable order #645
Conversation
There was a problem hiding this comment.
Thank you for your work.
I see the formatting failed, can you run make format to autofix all instances?
Gentoo developers are rejecting routine version bumps for ebuild variables being defined in a different order than skel.ebuild. This new lint ensures pkgcheck identifies these problems before we waste developer time.
Since this is outside of normal enforcement levels, I've requested QA team input on the check idea. Therefore approval of the idea might take time.
TODO
Both of the following tests have regressions that need a new solution: tests/scripts/test_pkgcheck_scan.py::TestPkgcheckScan::test_filter_latest FAILED tests/scripts/test_pkgcheck_scan.py::TestPkgcheckScan::test_scan_quiet FAILED
The conflict here stems from: pkgcore/pytest/plugin.py create_ebuild() which creates ebuilds that don't match the same skel.ebuild variable order that Gentoo developers are enforcing on the tree.
I'm requesting feedback on how we should fix those tests. Should create_ebuild() be updated to enforce variable order? Or, should we rewrite those two tests to try and stop assuming there's a precisely defined number of errors shown or supressed (which will continue to change in the future as new lints are added or removed).
let's change the origin at pkgcore if it is simple, otherwise we should take deeper look at why it is hard.
Regarding tests, in spite of the massive diff, all that's been done is re-ordering the variables to avoid introducing new style warnings into existing tests.
Better matches the order defined in skel.ebuild A pre-requisite for pkgcore/pkgcheck#645
Better matches the order defined in skel.ebuild A prerequisite for pkgcore/pkgcheck#645
|
Requested changes have been incorporated, and a pull request against pkgcore opened here: pkgcore/pkgcore#425 to resolve the problems with the last two tests. |
@anthonyryan1 for my own knowledge, can you link an example? |
|
Certainly! |
Better matches the order defined in skel.ebuild A prerequisite for pkgcore/pkgcheck#645 Signed-off-by: Anthony Ryan <[email protected]>
Better matches the order defined in skel.ebuild A prerequisite for pkgcore/pkgcheck#645 Signed-off-by: Anthony Ryan <[email protected]> Closes: #425 Signed-off-by: Arthur Zamarin <[email protected]>
|
Tests are green now that pkgcore is updated. Since I know discussion takes a while, I'll rebase this (and remove the now unnecessary changes to *DEPEND order in tests) after the discussion is complete. Just let me know once we have a verdict on it. |
|
OK, we've got enough agreements, so I think we can merge it. Can you please rebase over master (sorry, for adding conflicts for you) Edit: please also add a sign off to the commit |
anthonyryan1
force-pushed
the
master
branch
2 times, most recently
from
fb9f179
to
34374ad
Compare
|
Ready for CI approval to ensure nothing regressed during the rebase. Update: Will figure out what regressed and get this fixed shortly. Update 2: Fixed. Had to add some extra boilerplate for compatibility after #656 |
Gentoo developers are rejecting routine version bumps for ebuild variables being defined in a different order than skel.ebuild. This new lint ensures pkgcheck identifies these problems before we waste developer time. Regarding tests, in spite of the massive diff, all that's been done is re-ordering the variables to avoid introducing new style warnings into existing tests. Signed-off-by: Anthony Ryan <[email protected]>
|
You improved the Perfectionist World 😄 |
Gentoo developers are rejecting routine version bumps for ebuild variables being defined in a different order than skel.ebuild.
This new lint ensures pkgcheck identifies these problems before we waste developer time.
The following pkgcore pull request was submitted as a prerequisite: pkgcore/pkgcore#425
Specifically to address false positives in the following tests:
tests/scripts/test_pkgcheck_scan.py::TestPkgcheckScan::test_filter_latest FAILED
tests/scripts/test_pkgcheck_scan.py::TestPkgcheckScan::test_scan_quiet FAILED
Regarding tests, in spite of the massive diff, all that's been done is re-ordering the variables to avoid introducing new style warnings into existing tests.