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

Refactor code.yml Action #749

Closed
wants to merge 8 commits into from
Closed

Refactor code.yml Action #749

wants to merge 8 commits into from

Conversation

arichiv
Copy link
Member

@arichiv arichiv commented Jan 29, 2025

The prior code.yml action was validating no RWS sets and running tests, and it did this on PRs, main, and CRON.

We want to validate all sets on main/CRON but not block PRs.

code.yml now just runs tests on PRs and main.
json.yml was renamed json_pr.yml with no changes.
json_main.yml was added, and validates all RWS sets on main and CRON.

We should just check new sets on JSON.yml as it prints errors for the PR author to fix, but we should ensure this job covers the full file since it runs on master too.
@cfredric
Copy link
Collaborator

This makes it possible for unrelated timeouts/failures to block a PR submission, since PRs are validating the well-known files of the full set, right? I think it'd be better to make the PR workflows only validate things that are related to the PR itself.

@cfredric
Copy link
Collaborator

We're also going to have to enhance the script itself to handle the legacy well-known file location, since the older set entries don't serve from related-website-set.json.

@arichiv arichiv changed the title Validate the full set on code.yml jobs Refactor code.yml Action Jan 29, 2025
Copy link

The RWS JSON was successfully validated!

THIS PR CONTAINS SYSTEM CHANGES PLEASE REVIEW WITH CARE!

@sjledoux
Copy link
Collaborator

I don't think that the new workflow this is adding,json_main.yml, is needed/helpful. Due to old RWS members that are not currently compliant with the RWS submission requirements, check_sites.py will consistently fail when passed the full JSON list.

@arichiv
Copy link
Member Author

arichiv commented Jan 29, 2025

I added #750 to resolve errors in old sets. I think it's worth checking them so that we know if they drift out of compliance for some reason, but exemptions may have to be added for some legacy entries.

@arichiv arichiv closed this Jan 30, 2025
@arichiv arichiv deleted the arichiv-patch-1 branch January 30, 2025 19:04
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.

4 participants