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

sort OptionalObsoleteFiles blocks #896

Closed
wants to merge 8 commits into from

Conversation

brooksdavis
Copy link
Contributor

OptionalObsoleteFiles.inc was almost, but not quite sorted which made it hard to figure out where to put things. There seems to have been a mix of more sorting errors, things that didn't move when the option controlling them changed, and odd choices for locations.

I've broken this up into a set of commits moving one or two blocks at a time in hopes that if people have merge difficulty they will be able to merge individual changes where it's easy to see what the conflict is.

@bsdimp
Copy link
Member

bsdimp commented Nov 13, 2023

A quick look suggests this is sane.
Do we document the desired order at the top?

Using sort(1) order makes verification of the sort easier.
Presumably these were under MK_CDDL at some point, but these days
src.opts.mk takes care of setting them to "no" when MK_CDDL is.
These used to be grouped with MK_GCOV option or the like.
Options should be in sort order by primary option (usually, but not
always first in the if statement).
@brooksdavis
Copy link
Contributor Author

brooksdavis commented Nov 13, 2023

A quick look suggests this is sane. Do we document the desired order at the top?

I've added a comment to document the order (it's unfortunately awkward to enforce mechnically).

@brooksdavis brooksdavis reopened this Nov 13, 2023
OLD_FILES+=usr/share/examples/dma/mailer.conf
OLD_DIRS+=usr/share/examples/dma

.if ${MK_DMAGENT} == no
Copy link
Member

Choose a reason for hiding this comment

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

what happened here?

Copy link
Member

Choose a reason for hiding this comment

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

The old file doesn't have superfluous whitespace, and the new one doesn't seem to have either...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really don't know what's going on here. There aren't any changes to these DMA lines, but git insists on displaying things this way.

freebsd-git pushed a commit that referenced this pull request Nov 14, 2023
Using sort(1) order makes verification of the sort easier.

Reviewed by:	emaste, imp, netchild
Pull Request:	#896
freebsd-git pushed a commit that referenced this pull request Nov 14, 2023
Reviewed by:	emaste, imp, netchild
Pull Request:	#896
freebsd-git pushed a commit that referenced this pull request Nov 14, 2023
Presumably these were under MK_CDDL at some point, but these days
src.opts.mk takes care of setting them to "no" when MK_CDDL is.

Reviewed by:	emaste, imp, netchild
Pull Request:	#896
freebsd-git pushed a commit that referenced this pull request Nov 14, 2023
Reviewed by:	emaste, imp, netchild
Pull Request:	#896
freebsd-git pushed a commit that referenced this pull request Nov 14, 2023
These used to be grouped with MK_GCOV option or the like.

Reviewed by:	emaste, imp, netchild
Pull Request:	#896
freebsd-git pushed a commit that referenced this pull request Nov 14, 2023
Reviewed by:	emaste, imp, netchild
Pull Request:	#896
freebsd-git pushed a commit that referenced this pull request Nov 14, 2023
Reviewed by:	emaste, imp, netchild
Pull Request:	#896
freebsd-git pushed a commit that referenced this pull request Nov 14, 2023
Options should be in sort(1) order by primary option (usually, but not
always, first in the if statement).

Reviewed by:	emaste, imp, netchild
Pull Request:	#896
@brooksdavis brooksdavis deleted the oof-cleanup branch November 14, 2023 17:15
freebsd-git pushed a commit that referenced this pull request Dec 2, 2023
Using sort(1) order makes verification of the sort easier.

Reviewed by:	emaste, imp, netchild
Pull Request:	#896

(cherry picked from commit 4ca5df8)
freebsd-git pushed a commit that referenced this pull request Dec 2, 2023
Reviewed by:	emaste, imp, netchild
Pull Request:	#896

(cherry picked from commit e70ad27)
freebsd-git pushed a commit that referenced this pull request Dec 2, 2023
Presumably these were under MK_CDDL at some point, but these days
src.opts.mk takes care of setting them to "no" when MK_CDDL is.

Reviewed by:	emaste, imp, netchild
Pull Request:	#896

(cherry picked from commit a62cefb)
freebsd-git pushed a commit that referenced this pull request Dec 2, 2023
Reviewed by:	emaste, imp, netchild
Pull Request:	#896

(cherry picked from commit 16743f0)
freebsd-git pushed a commit that referenced this pull request Dec 2, 2023
These used to be grouped with MK_GCOV option or the like.

Reviewed by:	emaste, imp, netchild
Pull Request:	#896

(cherry picked from commit fb173fc)
freebsd-git pushed a commit that referenced this pull request Dec 2, 2023
Reviewed by:	emaste, imp, netchild
Pull Request:	#896

(cherry picked from commit 2b3bf27)
freebsd-git pushed a commit that referenced this pull request Dec 2, 2023
Reviewed by:	emaste, imp, netchild
Pull Request:	#896

(cherry picked from commit c90d060)
freebsd-git pushed a commit that referenced this pull request Dec 2, 2023
Options should be in sort(1) order by primary option (usually, but not
always, first in the if statement).

Reviewed by:	emaste, imp, netchild
Pull Request:	#896

(cherry picked from commit 4cd0f01)
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 2, 2024
Using sort(1) order makes verification of the sort easier.

Reviewed by:	emaste, imp, netchild
Pull Request:	freebsd/freebsd-src#896
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 2, 2024
Reviewed by:	emaste, imp, netchild
Pull Request:	freebsd/freebsd-src#896
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 2, 2024
Presumably these were under MK_CDDL at some point, but these days
src.opts.mk takes care of setting them to "no" when MK_CDDL is.

Reviewed by:	emaste, imp, netchild
Pull Request:	freebsd/freebsd-src#896
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 2, 2024
Reviewed by:	emaste, imp, netchild
Pull Request:	freebsd/freebsd-src#896
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 2, 2024
These used to be grouped with MK_GCOV option or the like.

Reviewed by:	emaste, imp, netchild
Pull Request:	freebsd/freebsd-src#896
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 2, 2024
Reviewed by:	emaste, imp, netchild
Pull Request:	freebsd/freebsd-src#896
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 2, 2024
Reviewed by:	emaste, imp, netchild
Pull Request:	freebsd/freebsd-src#896
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Mar 2, 2024
Options should be in sort(1) order by primary option (usually, but not
always, first in the if statement).

Reviewed by:	emaste, imp, netchild
Pull Request:	freebsd/freebsd-src#896
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants