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

Allowed JAR saving if proceedOnError is specified #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

256p
Copy link

@256p 256p commented Sep 15, 2021

Added proceedOnError() check in order to allow JAR generation if -proceedOnError argument is passed.

@kriegaex
Copy link
Contributor

@aclement, should this one be merged? I am not sure what it does.

For some reason I thought it was merged already, maybe because the previous PR #87 was closed. I even mentioned it in the 1.9.8 release notes, but am going to remove it for now.

@aclement
Copy link
Contributor

The comment right above the change is /* Ensure we don't write an incomplete JAR bug-71339 */ so I'd say that bug at least needs reading. So I don't think it is an automatic merge, no.

@256p
Copy link
Author

256p commented Jan 18, 2022

For some reason I thought it was merged already, maybe because the previous PR #87 was closed.

I closed the first PR because I have mistaken and my Eclipse Contributor Agreement was not validated.

@256p
Copy link
Author

256p commented Jan 18, 2022

The comment right above the change is /* Ensure we don't write an incomplete JAR bug-71339 */ so I'd say that bug at least needs reading. So I don't think it is an automatic merge, no.

Maybe I am wrong but as I understand this bug was related to the build without -proceedOnError argument. And yes in some cases it might result in corrupted outjar. But isn't -proceedOnError argument supposed to return results regardless of errors?

@kriegaex
Copy link
Contributor

kriegaex commented Mar 25, 2022

as I understand this bug was related to the build without -proceedOnError argument. And yes in some cases it might result in corrupted outjar. But isn't -proceedOnError argument supposed to return results regardless of errors?

@aclement, I think @256p has a point here. How about logging a warning about a possibly corrupted JAR in case of -proceedOnError, but then continuing to write the file?

@256p, can you please provide a reasonable test case for which it makes sense to continue? A negative test which corrupts the file would also be welcome. That would help to move this issue along, because I think this deserves test coverage. If your test case is not automated, because you have trouble understanding the structure and configuration of AspectJ tests (which would not surprise me), at least reproducible examples, as minimal as possible, would be a good starting point for us to automate it. Depending on how important your authorship is for this PR, I could commit on top your commit of it or start from scratch.

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