Skip to content

Comments

Fix detection of changed files in CI#9035

Merged
zanieb merged 1 commit intomainfrom
debug/determine-changes
Dec 7, 2023
Merged

Fix detection of changed files in CI#9035
zanieb merged 1 commit intomainfrom
debug/determine-changes

Conversation

@zanieb
Copy link
Member

@zanieb zanieb commented Dec 7, 2023 •
edited
Loading

These were literals instead of expressions... and were consequently not evaluated.

Fixes bug from #8225

@zanieb zanieb force-pushed the debug/determine-changes branch from 73bbb99 to db6f649 Compare December 7, 2023 02:52
@zanieb zanieb marked this pull request as ready for review December 7, 2023 02:53
@zanieb zanieb mentioned this pull request Dec 7, 2023
Merged
@MichaReiser
Copy link
Member

MichaReiser commented Dec 7, 2023

I'm not sure if that's the root cause because the checks did ran in the last few days and the if documentation mentions that omitting the ${{ }} is okay except when the expression starts with !.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2023

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@zanieb zanieb added the ci Related to internal CI tooling label Dec 7, 2023
@zanieb
Copy link
Member Author

zanieb commented Dec 7, 2023 •
edited
Loading

@MichaReiser they didn't run on this branch before the change, but did after 🤷‍♀️ perhaps a change on GitHub's end

@zanieb
Copy link
Member Author

zanieb commented Dec 7, 2023

Also, I ran the workflow with debug logs and the "Determine changes" workflow is not at fault

@zanieb zanieb merged commit 06c9f62 into main Dec 7, 2023
@zanieb zanieb deleted the debug/determine-changes branch December 7, 2023 03:14
@zanieb
Copy link
Member Author

zanieb commented Dec 7, 2023

Hm but it does seem to be at fault over in the other branch https://github.com/astral-sh/ruff/actions/runs/7122120523/job/19395730196

@zanieb
Copy link
Member Author

zanieb commented Dec 7, 2023 •
edited
Loading 

 M	Cargo.toml
  ##[debug]All diff files: {"A":[],"C":[],"D":[],"M":["Cargo.toml"],"R":[],"T":[],"U":[],"X":[]}
  All Done!
  ::endgroup::
::group::changed-files-yaml-linter
changed-files-yaml-linter
::group::changed-files-yaml-formatter
changed-files-yaml-formatter
::group::changed-files-yaml-code
changed-files-yaml-code
  ##[debug]All filtered diff files for code: {"A":[],"C":[],"D":[],"M":[],"R":[],"T":[],"U":[],"X":[]}
  ##[debug]Dir names include file patterns: []
  ##[debug]Added files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]Copied files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]Modified files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]Renamed files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]Type changed files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]Unmerged files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]Unknown files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]All changed and modified files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]All changed files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]All other changed files: {"paths":"Cargo.toml","count":"1"}
  ##[debug]other_changed_files: ["Cargo.toml"]
  ##[debug]Dir names include file patterns: []
  ##[debug]All modified files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]other_modified_files: ["Cargo.toml"]
  ##[debug]Dir names include file patterns: []
  ##[debug]Deleted files: {"paths":"","count":"0"}
  ##[debug]Dir names include file patterns: []
  ##[debug]other_deleted_files: []
  ..
  ##[debug]Set output code_any_changed = false

@zanieb
Copy link
Member Author

zanieb commented Dec 7, 2023 •
edited
Loading

Ah it's because the M Cargo.toml gets included in the linter changes

@zanieb zanieb mentioned this pull request Dec 7, 2023
Merged
zanieb added a commit that referenced this pull request Dec 7, 2023
@zanieb
Fix determine changes detection of "code" changes (#9038)
d22ce53 
Replaces #9035
Fixes #8225

The issue appears to be that `*/**` was used instead of `**/*` which did
not match _any_ changed file as desired
@ofek
Copy link
Contributor

ofek commented Dec 7, 2023

Now that the root cause has been fixed you may want to remove the ${{ }} again to look idiomatic

@zanieb
Copy link
Member Author

zanieb commented Dec 7, 2023

Eh we use ${{ }} for every other if statement in that file. I'd prefer to be consistent.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

ci Related to internal CI tooling

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zanieb @MichaReiser @ofek