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

Eclipse apt fixes #716

Merged
merged 2 commits into from
Feb 1, 2019
Merged

Eclipse apt fixes #716

merged 2 commits into from
Feb 1, 2019

Conversation

FroMage
Copy link
Member

@FroMage FroMage commented Feb 1, 2019

Temporary workarounds for #713

Copy link
Member

@dmlloyd dmlloyd left a comment

Choose a reason for hiding this comment

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

Looks OK. We'll fix this properly later.

@@ -149,7 +151,7 @@ void doFinish(RoundEnvironment roundEnv) {
return;
}
final URI uri = tempResource.toUri();
tempResource.delete();
// tempResource.delete();
Copy link
Member

Choose a reason for hiding this comment

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

This looks fine, it's an empty file anyway and should do no harm I would think if it stays around.

Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to keep it around? Can't we delete it at some point?

Copy link
Member

Choose a reason for hiding this comment

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

@FroMage out of curiosity, could you explain why we need to keep the file? A comment might help if we need to keep it this way.

Copy link
Member Author

Choose a reason for hiding this comment

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

We're not keeping that file: Eclipse throws if you try to delete it, and we write to it just below.

Copy link
Member

Choose a reason for hiding this comment

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

Well we don't really write to it, we just use it to find the path and get the parent path. So we can still delete it if we find a way to make Eclipse happy about it, though like I said it's not really too important.

@dmlloyd
Copy link
Member

dmlloyd commented Feb 1, 2019

OK to merge once CI reports in.

@gsmet gsmet added this to the 0.8.0 milestone Feb 1, 2019
@FroMage
Copy link
Member Author

FroMage commented Feb 1, 2019

Looks like it failed, can you tell me why? I don't have permissions to log in the CI.

@gsmet
Copy link
Member

gsmet commented Feb 1, 2019

@FroMage not related to this PR, it fails very late when testing the CLI tools.

@FroMage
Copy link
Member Author

FroMage commented Feb 1, 2019

Ah. Damn. So… what now?

@Sanne
Copy link
Member

Sanne commented Feb 1, 2019

Ah. Damn. So… what now?

you ignore it & merge

@Sanne Sanne merged commit 21f5a16 into quarkusio:master Feb 1, 2019
@Sanne Sanne deleted the eclipse-apt-fixes branch February 1, 2019 16:22
@FroMage
Copy link
Member Author

FroMage commented Feb 1, 2019

Fair enough, thanks :)

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.

4 participants