Skip to content

CI: Determine if source files were changed before building#116963

Open
Repiteo wants to merge 1 commit intogodotengine:masterfrom
Repiteo:ci/changed-files-advanced
Open

CI: Determine if source files were changed before building#116963
Repiteo wants to merge 1 commit intogodotengine:masterfrom
Repiteo:ci/changed-files-advanced

Conversation

@Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Mar 2, 2026

Implements specialization logic on the new changed-files action. This is already utilized by the godot-cpp repository to determine what source files changed, which is what I drew inspiration from. However, our repo is a fair bit more complex, so the sources needed to be expanded to cover our bases. This will mean that files only changing documentation or files that have negligible buildsystem impact will not result in GitHub Action resources. The two exceptions to this that will build regardless of changed files are:

  • A run was initiated by workflow_dispatch
  • If ref_name is equal to the default branch's name (aka: a push event to the main branch)

@Repiteo Repiteo added this to the 4.x milestone Mar 2, 2026
@Repiteo Repiteo requested a review from a team as a code owner March 2, 2026 16:02
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Code looks good to me.

@Repiteo Repiteo modified the milestones: 4.x, 4.7 Mar 2, 2026
@AThousandShips
Copy link
Member

I think this is a good improvement, it does mean that we have to keep an eye on pure documentation changes to make sure they don't do invalid changes like changing the order (and I think using some invalid syntax like & instead of &, I don't think that's enforced by the xml check), as we only get checks for that in when doing the check for class reference

It's a minor aspect but just good to make sure to track, as it's not possible to do workflow dispatch on PRs

@Repiteo Repiteo force-pushed the ci/changed-files-advanced branch from 5f68931 to 2820bc9 Compare March 2, 2026 17:32
@Repiteo
Copy link
Contributor Author

Repiteo commented Mar 2, 2026

Hmm, I hadn't considered invalid changes in documentation. It'd be nice if that could somehow be integrated as part of the xml checker, because having to build the entire engine to account for that would be rough.

@AThousandShips
Copy link
Member

AThousandShips commented Mar 2, 2026

It'd be impossible to do the full checks without the editor, for example someone just adding an invalid entry, that would be trivial to detect though

Sorting entries would be more trivial to detect, but not sure how easy it would be to implement, also don't know if the syntax part is covered by the static check

To clarify though: I think this is still a worthwhile change

@Repiteo
Copy link
Contributor Author

Repiteo commented Mar 2, 2026

Well sorting wouldn't be a concern in this context, would it? It's not like a user would be adding an entirely new section if there aren't any associated code changes

@AThousandShips
Copy link
Member

AThousandShips commented Mar 2, 2026

It's unlikely yes, and should be easy to discover manually

Edit: We also lose the ability in such cases to notice invalid changes to default values, so just something to keep an eye on, like a few invalid PRs in the past

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants