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

Permit uploading artifacts for aborted builds #132

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

wheelerlaw
Copy link

Right now there is a configuration option for overriding the default behavior that prevents uploading artifacts when the build fails. However nonesuch option exists for overriding the same behavior preventing uploading artifacts for aborted builds.

This adds that option so that for aborted builds artifacts can be pushed to S3.

/**
* Do not publish the artifacts when build is aborted
*/
public boolean noUploadOnAborted;
Copy link
Member

Choose a reason for hiding this comment

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

To the best of my understanding, this needs to be true in order to preserve the backwards compatibility. However, existing configurations will be migrated so this gets set to false (default value for boolean) causing it to change semantics.

Better use Boolean here and do the null -> true conversion in readResolve: https://wiki.jenkins.io/display/JENKINS/Hint+on+retaining+backward+compatibility#Hintonretainingbackwardcompatibility-Scenario:Addinganewfield

Copy link
Author

@wheelerlaw wheelerlaw Aug 11, 2020

Choose a reason for hiding this comment

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

Just made those changes. Let me know what you think

@wheelerlaw
Copy link
Author

wheelerlaw commented Sep 3, 2020

Any update on getting this merged?

@wheelerlaw
Copy link
Author

@olivergondza bump

@lslebodn
Copy link

@daniel-beck @Jimilian @varyvol

Could you help merging/review?

@olivergondza
Copy link
Member

@varyvol, can you please take a look? Thanks!

@lslebodn
Copy link

@varyvol Would you mind to check the PR ?

@lslebodn
Copy link

@alecharp I can see you did latest commits in the repository.
Would you be so kind and could you review the patch?

@wheelerlaw
Copy link
Author

@alecharp @olivergondza bumping this again because we are still in need of this functionality

@lslebodn
Copy link

lslebodn commented Nov 5, 2021

@rsandell would you mind to review the changes?
It would be great to have it in the next release 0.12.x

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.

3 participants