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

feat(linter): section header validation #2329

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

Conversation

wdhdev
Copy link
Contributor

@wdhdev wdhdev commented Dec 11, 2024

This PR just fixes a few formatting issues (mostly just errors) in the PSL, as well as updating a couple links (mainly ones that were invalid/missing).

@simon-friedberger
Copy link
Contributor

Can you replace "grandfathered" with "legacy" or something if that's what you mean here?

@wdhdev
Copy link
Contributor Author

wdhdev commented Dec 11, 2024

Done. By the way, the tests are only failing due to GitHub rate limits, there isn't anything actually wrong with the PR, it should be fine to merge. For some reason it seems the tests attempt to pull a ton of PRs, not sure why however.

@simon-friedberger
Copy link
Contributor

Ah, I think that is an issue for @danderson.
I assume that, since this is a clean-up PR and it touches a lot of sections, the script is trying to get all the PRs for those sections to check the TXT records.

@wdhdev
Copy link
Contributor Author

wdhdev commented Dec 11, 2024

Are we able to merge in the meantime and create a follow up issue?

@wdhdev wdhdev changed the title fix formatting issues fix formatting issues + remove gon.pk Dec 11, 2024
@wdhdev wdhdev changed the title fix formatting issues + remove gon.pk fix formatting issues + remove gon.pk Dec 11, 2024
@wdhdev wdhdev changed the title fix formatting issues + remove gon.pk feat(linter): section header validation Dec 11, 2024
@wdhdev
Copy link
Contributor Author

wdhdev commented Dec 11, 2024

@simon-friedberger Linter has been updated to now validate section headers. I forgot the linter was actually in Python, not Go, so that made it much easier!

This PR is ready to merge unless you have any other comments. (The failing test is only due to the GitHub rate limits as I said before)

@simon-friedberger
Copy link
Contributor

Damn, sorry. This is the legacy linter that we are replacing with the Go one. We don't want to update that anymore.

@wdhdev
Copy link
Contributor Author

wdhdev commented Dec 12, 2024

Ah. Should we remove this linter all together?

I'll see if I can implement the same functionality into the Go linter.

@simon-friedberger
Copy link
Contributor

Yes, we should remove it. It's just here until someone puts in the work to make sure psltool covers the same things.

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