Skip to content

Skip deleted package files in CI. #560

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

Merged
merged 1 commit into from
Apr 22, 2025
Merged

Skip deleted package files in CI. #560

merged 1 commit into from
Apr 22, 2025

Conversation

mmuetzel
Copy link
Member

Fixes #545.

@mmuetzel mmuetzel marked this pull request as draft April 16, 2025 13:21
@mmuetzel mmuetzel marked this pull request as ready for review April 16, 2025 13:23
@pr0m1th3as
Copy link
Member

I am deploying the workflow on my local repository to test this. Will report back

@pr0m1th3as
Copy link
Member

Yep!! It works properly.

@pr0m1th3as pr0m1th3as requested review from siko1056 and pr0m1th3as and removed request for pr0m1th3as April 16, 2025 17:08
@pr0m1th3as
Copy link
Member

@mmuetzel I assume you've tested it this as well. Should we wait for @siko1056 before merging or you think that we suffice?

@mmuetzel
Copy link
Member Author

I didn't test this much apart from feeding some input into that bash snippet locally.

I'd prefer if @siko1056 could have a look, too. There's also no rush for this. So, imho we could keep this open for a week or so.
If Kai didn't have time until then, we could still take a deeper look and decide on whether to merge...

@pr0m1th3as
Copy link
Member

Ok. Let's wait for @siko1056 's input as well. I did test it on a deployed version in my personal GitHub account and it seems to work (after pushing a commit to remove a yaml file, after a few refreshes, the Package Index wasn't listing the removed package, but I am not 100% confident about what the functionality should/would look like. I am not sure, whether merging a PR behaves differently, because on the actual deployment (this repo) we cannot push a commit, it has to be pulled as a PR. So let's wait a bit. packajoozle has been removed anyway.

@siko1056 siko1056 added bug Something isn't working enhancement New feature or request labels Apr 16, 2025
@siko1056
Copy link
Member

Thank you for this PR, I will try it later this week.

@siko1056
Copy link
Member

siko1056 commented Apr 19, 2025

Works like a charm @mmuetzel . Please merge 🙏

I made a local test commit that removes two packages, changes two packages and adds one package. It detects it all and would run the checks on the modified and new package only 👍

bash check.sh 
M       packages/bsltl.yaml
D       packages/chartjs.yaml
M       packages/image.yaml
D       packages/json.yaml
A       packages/new_package.yaml
CHANGED_PACKAGES=" bsltl  image  new_package "

@mmuetzel
Copy link
Member Author

Thank you both for the review.

@mmuetzel mmuetzel merged commit 7da8455 into gnu-octave:main Apr 22, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI does not handle case of unlisting a package
3 participants