-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Schema Publish workflow #4162
Schema Publish workflow #4162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused because this seems to get triggered for any commit to the main
branch rather than filtering on the schemas, and it does not seem to check that the $GITHUB_SHA
commit includes at least one schema either.
But I'm not very familiar with either github actions or bash scripting, so what am I missing here?
if [ "$lastCommitDate" \> "$maxDate" ]; then | ||
maxDate=$lastCommitDate | ||
fi | ||
datesHash["$schema"]=$maxDate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want each datesHash["$schema"]
to have $schema
's date, which would be $lastCommitDate
and not $maxDate
? It's not clear to me what $maxDate
is intended to do here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The schemas are visited in the order of their dependency:
- If
meta
changes,dialect
needs to be updated. Ifdialect
itself has changed, its new date is the maximum of the two change dates. - If
dialect
changes,schema
needs to be updated. Ifschema
itself has changed, its new date is the maximum of the two change dates. - If
schema
changes,schema_base
needs to be updated. Ifschema_base
itself has changed, its new date is the maximum of the two change dates.
This is what the loop & $maxDate
do because we currently have a linear dependency chain. If we get more files and non-linear dependencies, this has to be revisited.
This action needs to run if
We could do that with a Given that the workflow only creates a pull request for publishing the schemas if it actually produces changes compared to |
According to GitHub Docs on the
or
This seems to indicate that if I
Then the It seems safer to refresh the JSON files based on the date of the last change to the corresponding YAML file and its dependent YAML files. |
Yes if we're not filtering the trigger then we need to filter the commits. |
We use the date of the last commit for each file to produce its relevant date, in combination with the new dates of all files it references. The workflow then creates a pull request for publishing all new/changed/renamed/deleted files. Git figures out which files actually need to be in the pull request, and if there are none, no pull request is created. This is the same mechanism we use for publishing the spec HTML files. |
@ralfhandl sorry for not finishing up with this yesterday- I was trying to figure out the code and suggest some comments to put in it because I get what you're saying but for some reason I am finding the code really hard to follow. I think the name |
Changed the name to something hopefully less confusing and added a few comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I find this easier to follow (and I now realize that my previous comment was misleading as I was confused by maxDate
more than lastDate
), but I'm finding it hard to come up with an improvement so I suppose I'll just stop standing in the way. I don't know why I find this so baffling.
Follow-up to
Replace WORK-IN-PROGRESS placeholders with date of last change of YAML source:
meta
changes, republish all four filesdialect
changes, republish it,schema
, andschema_base
schema
changes, republish it andschema_base
schema_base
changes, republish itExample PR created by this workflow