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

Switch ApacheConf to maintained upstream #7143

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nisbet-hubbard
Copy link

@nisbet-hubbard nisbet-hubbard commented Dec 1, 2024

Description

Discussed in #7135.

Checklist:

@nisbet-hubbard nisbet-hubbard requested a review from a team as a code owner December 1, 2024 05:27
Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

Please use the documented method of changing grammars.

@nisbet-hubbard
Copy link
Author

Done!

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

The script should have also created a yaml file containing the grammar licence. We need that file too. If it wasn't created or you've cleaned up, running just this line should do the trick:

bundle exec licensed cache -c vendor/licenses/config.yml

@lildude
Copy link
Member

lildude commented Dec 2, 2024

Oooof. Looks like @michidk has renamed the typst grammar repo. I've opened #7145 to track this.

@lildude
Copy link
Member

lildude commented Dec 3, 2024

Oooof. Looks like @michidk has renamed the typst grammar repo. I've opened #7145 to track this.

Resolved now. The test failures are legit. Looks like you may not have used the script/add-grammar command as documented. Please use that script to replace the grammar.

@nisbet-hubbard
Copy link
Author

Looks like you may not have used the script/add-grammar command as documented.

That's exactly what I did. What else is missing?

@lildude
Copy link
Member

lildude commented Dec 7, 2024

That's exactly what I did. What else is missing?

See the test failures. They indicate the grammar may not have been swapped using the script as it should have prevented all of those issues as it updates all the files reported in those errors.

@nisbet-hubbard
Copy link
Author

So what happened appears to be that after running the script, three files got automatically staged whilst the licence and grammars.yml had to be manually staged.

These I’ve now committed.

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

Whoops. The grammars.yaml file has waaaaaaay too many changes.

@nisbet-hubbard
Copy link
Author

Sorry, fixed that!

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

The vendor/README.md should have also been updated by the script.

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.

2 participants