Skip to content

Marked plugin as thread safe#213

Merged
lsf37 merged 1 commit intojflex-de:masterfrom
dheid:master
Apr 13, 2017
Merged

Marked plugin as thread safe#213
lsf37 merged 1 commit intojflex-de:masterfrom
dheid:master

Conversation

@dheid
Copy link
Contributor

@dheid dheid commented Apr 10, 2017

Updated to maven-plugin-api 3.0.5 and introduced Mojo annotations. Marked JFlexMojo as thread-safe. Closes #211

@sarowe
Copy link
Contributor

sarowe commented Apr 10, 2017

This looks great, thanks Daniel! But as Gerwin mentioned over on #211, there are known non-thread-safe things that need to be dealt with, e.g. converting static options to instance members, before we can declare the plugin thread-safe.

@dheid
Copy link
Contributor Author

dheid commented Apr 11, 2017 via email

@lsf37
Copy link
Member

lsf37 commented Apr 11, 2017

I think the annotations are a good idea, happy to have them, if we mark it as non thread-safe for now. @sarowe do you agree?

@sarowe
Copy link
Contributor

sarowe commented Apr 11, 2017

I think the annotations are a good idea, happy to have them, if we mark it as non thread-safe for now. @sarowe do you agree?

+1

@dheid
Copy link
Contributor Author

dheid commented Apr 12, 2017 via email

@lsf37 lsf37 merged commit 3eae6b6 into jflex-de:master Apr 13, 2017
@lsf37
Copy link
Member

lsf37 commented Apr 13, 2017

Now merged and marked as not thread safe (for now) as discussed in 104b219

Thanks for the contribution!

</issueManagement>
<prerequisites>
<maven>3.0.1</maven>
<maven>${maven.version}</maven>
Copy link
Member

Choose a reason for hiding this comment

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

We need a very recent version of Maven to compile JFlex due to a bug in maven-shade-plugin.

Are you sure we actually need the same to execute the plugin? It sounds to me that 3.0 should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maven version 3 should really be enough. The annotation were introduced back then.

@regisd
Copy link
Member

regisd commented Nov 7, 2017

Thanks for updating the plugin to use Java annotations rather than invalid javadoc tags.

@dheid
Copy link
Contributor Author

dheid commented Nov 8, 2017

You are welcome!

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