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

Remove legacy top-scope syntax #1205

Merged
merged 1 commit into from Feb 20, 2024
Merged

Conversation

smortex
Copy link
Member

@smortex smortex commented Nov 23, 2023

No description provided.

@smortex smortex self-assigned this Nov 23, 2023
@kenyon
Copy link
Member

kenyon commented Nov 23, 2023

Just changes in CHANGELOG and HISTORY. Maybe we don't want to change these?

@smortex
Copy link
Member Author

smortex commented Nov 23, 2023

Just changes in CHANGELOG and HISTORY. Maybe we don't want to change these?

I am not 100% sure about it. As you probably guessed, I automated these fixes on all voxpupuli modules and opened draft-PRs to review them and have CI confirm I don't break everything 😄

I started this effort because I saw that a coworker who is starting to use Puppet got really confused about these :: he saw in some places and not in other ones. So I try to make it easier for newcomers by having a more consistent syntax.

There are a few PRs like this one where only the "doc" (e.c. CHANGELOG) is updated. At first I though we can ignore these and close the PR, and then I though that keeping them was going against the idea of unification I was following… IMHO, fixing README.md is a must-do, so fixing CHANGELOG.md to match the syntax is probably fine.

This is of course open for discussion because I am also not excited by rewriting old changelog entries.

@kenyon
Copy link
Member

kenyon commented Nov 23, 2023

Yes, agreed that it's a good idea to eliminate legacy syntax in as many places as possible so newcomers don't get confused. My only concern was that these changes could get undone by the changelog generator, but these ones don't look autogenerated, which is why I approved.

@smortex smortex marked this pull request as ready for review November 23, 2023 20:45
@zilchms zilchms merged commit 1a35468 into master Feb 20, 2024
9 checks passed
@zilchms zilchms deleted the remove-legacy-top-scope-syntax branch February 20, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants