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

Allow fails in Remove large packages #8

Merged
merged 4 commits into from
Sep 29, 2023
Merged

Conversation

kfir4444
Copy link
Contributor

@kfir4444 kfir4444 commented Jun 15, 2023

Added || true at the end of large file removal, so that if one step of it fails, the clean-up job would continue.

@jlumbroso
Copy link
Owner

Thank you very much @kfir4444 for suggesting this improvement!

I wanted to ask your opinion:

  • Do you think there are any scenarios where the user may want this to fail? (Should this be a flag.)
  • Do you think that it would be useful to print something when the command fails, or do you think the log output produced by the commands is sufficient?

Also, do you know which packages you have had an issue with?

My impression is that this can be merged without any impact to existing users, but I would like to double check you believe so too. Thank you!

@kfir4444
Copy link
Contributor Author

Thanks for the replay @jlumbroso! 😃
Regarding your first question, I can think of some scenarios where this action could be used as a test, so a failure here could be important, but I think that the general use case only benefits from not stopping a CI due to failed deletion, which hinders development.
Well, we use this github action in (at least) one of our repos and got some CI fails (example here) due to a false return code in ubuntu. I think that the users can tolerate a difference of a few mb's extra, but have a CI fail a few times due to uninstallation error may discourage users to use this tool. Also, I recall talking to a colleague and postulating that this issue may had something to do with the new ubuntu release and additional packages still being adapted to it (could not be verified, though), so this PR (which is already implemented in our software for a while using my fork) gave us the opportunity to continue developing even in a (somewhat) unstable environment associated with open source code (which we absolutely love!).

@pratik-techholding
Copy link

Solution of @kfir4444 looks good can we approve this PR please many user facing issues

@kfir4444
Copy link
Contributor Author

Thanks @pratik-techholding!
I do not think we should hate the code review process, if you want, you can use my clone until this PR is merge. I can ping you when it's merged 😄

@AurevoirXavier
Copy link

Maybe this could fix #9.

Copy link
Contributor

@ChrisCarini ChrisCarini left a comment

Choose a reason for hiding this comment

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

🎉 LGTM! Thank you for this change!

I'd be happy if this PR is merged as-is to simply unblock about a dozen or so of my repos' CI runs. The suggestion simply addresses @jlumbroso's question in their comment here; IMO, having messages printed for these command failures a 'nice-to-have', but not required. I'd rather see my CI jobs unblocked sooner rather than later.

@jlumbroso - by allowing this PR to be merged in and released, the below issues / PRs would be closed / addressed:

Thoughts on getting this merged in to unblock the many CI runs that are affected by issues this PR addresses?

action.yml Outdated Show resolved Hide resolved
@yinweisu
Copy link

yinweisu commented Aug 17, 2023

Hi, when do we expect to get the fix merged in? This issue is breaking my CI too

@ChrisCarini
Copy link
Contributor

Hi @jlumbroso - do you have any subsequent concerns and/or blockers to getting this PR merged?

If you need extra help maintaining this repository, I'd be happy to volunteer to assist (I have experience with authoring & maintaining several GHAs); just let me know.

@kfir4444
Copy link
Contributor Author

@jlumbroso as @ChrisCarini, I can also help with maintenance 😄

@kfir4444
Copy link
Contributor Author

For everyone else struggling with issues, you can use my fork for now (this is what we do in our repos) until this chage becomes official. I can tag you here when this PR is merged so you can return to the original.
@AurevoirXavier @yinweisu @ChrisCarini

@causten
Copy link

causten commented Sep 16, 2023

Thanks for the replay @jlumbroso! 😃 Regarding your first question, I can think of some scenarios where this action could be used as a test, so a failure here could be important, but I think that the general use case only benefits from not stopping a CI due to failed deletion, which hinders development.

Don't you get for free the error check if you support "continue-on-error: true" in the action?

@Stephane-Ag
Copy link

This would also fix #4. I assume @jlumbroso has just not had a chance to look at this.

Co-authored-by: Chris Carini <6374067+ChrisCarini@users.noreply.github.com>
@jlumbroso
Copy link
Owner

Sorry for the delay. @ChrisCarini Thank you for addressing my concerns with your change.

@jlumbroso jlumbroso merged commit f099676 into jlumbroso:main Sep 29, 2023
@ChrisCarini
Copy link
Contributor

Sorry for the delay.

No worries!

@ChrisCarini Thank you for addressing my concerns with your change.

Of course! FWIW, my offer to help you maintain this repo (ref: #8 (comment) ) (approving/merging PRs, releasing new versions, etc) still stands. :) Just let me know! Happy to help.

ChrisCarini added a commit to ChrisCarini/github-repo-files-sync that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886d.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/sample-intellij-plugin that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/automatic-github-issue-navigation-configuration-jetbrains-plugin that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/git-push-reminder-jetbrains-plugin that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/intellij-code-exfiltration that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/intellij-notification-sample that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/iris-jetbrains-plugin that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/jetbrains-auto-power-saver that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/jetbrains-sdk-cleaner that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/logshipper-intellij-plugin that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/example-loc-plugin-config-plugin that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/loc-change-count-detector-jetbrains-plugin that referenced this pull request Sep 29, 2023
…rification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).
ChrisCarini added a commit to ChrisCarini/environment-variable-settings-summary-intellij-plugin that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#282)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/automatic-github-issue-navigation-configuration-jetbrains-plugin that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#188)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/git-push-reminder-jetbrains-plugin that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#184)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/sample-intellij-plugin that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#247)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/intellij-code-exfiltration that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#158)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/iris-jetbrains-plugin that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#204)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/jetbrains-auto-power-saver that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#231)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/example-loc-plugin-config-plugin that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#134)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/intellij-notification-sample that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#193)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/jetbrains-sdk-cleaner that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#228)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/loc-change-count-detector-jetbrains-plugin that referenced this pull request Sep 30, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#250)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
ChrisCarini added a commit to ChrisCarini/logshipper-intellij-plugin that referenced this pull request Oct 4, 2023
…rification workflow."; Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0; Do not delete docker images from JetBrains compatibility GitHub Workflow. (#196)

* Revert "Use temporary GHA to clear more diskspace for IDE platform verification workflow."

This reverts commit a52886de94228e8250b852ba1c8075a35a4681bd.

Reverting because jlumbroso/free-disk-space#8 is now resolved and released (v1.3.0).

* Bump jlumbroso/free-disk-space from 1.2.0 to 1.3.0

* Do not delete docker images from JetBrains compatibility GitHub Workflow.
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.

8 participants