search

pkgcore / pkgcheck

pkgcore-based QA utility for ebuild repos
https://pkgcore.github.io/pkgcheck
BSD 3-Clause "New" or "Revised" License
35 stars 29 forks source link

VariableOrderWrong: Enforce skel.ebuild variable order #645

Closed anthonyryan1 closed 8 months ago

anthonyryan1 commented 8 months ago

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: https://github.com/pkgcore/pkgcore/pull/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.

anthonyryan1 commented 8 months ago

Requested changes have been incorporated, and a pull request against pkgcore opened here: https://github.com/pkgcore/pkgcore/pull/425 to resolve the problems with the last two tests.

ferringb commented 8 months ago

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.

@anthonyryan1 for my own knowledge, can you link an example?

anthonyryan1 commented 8 months ago

Certainly!

https://github.com/gentoo/gentoo/pull/34284#discussion_r1433301537

anthonyryan1 commented 8 months ago

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.

arthurzam commented 8 months ago

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 commented 8 months ago

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 https://github.com/pkgcore/pkgcheck/pull/656

kuraga commented 7 months ago

You improved the Perfectionist World :smile: