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.

Sign up for GitHub

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

Jump to bottom

Misc cleanup #449

Merged
merged 8 commits into from
Mar 15, 2023
Merged

Misc cleanup #449

merged 8 commits into from
Mar 15, 2023

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Mar 14, 2023

miscellaneous clean up

  • use maven logging where it makes sense for debug output
  • convert most resource management to try-with-resources
  • remove unused imports
  • as serialVersionUID to serialisable class (even if we never serialise it)
  • remove the direct usage of guava
  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

basil
basil approved these changes Mar 14, 2023
View reviewed changes
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Very nice!

src/main/java/org/jenkinsci/maven/plugins/hp/util/Utils.java Outdated Show resolved Hide resolved
src/main/java/org/jenkinsci/maven/plugins/hpi/AbstractHpiMojo.java Show resolved Hide resolved
@jtnord jtnord marked this pull request as draft March 14, 2023 17:37
@jtnord
Copy link
Member Author

jtnord commented Mar 14, 2023

back to draft - something is completely broken in the resolution of what goes into a jpi

jtnord
jtnord commented Mar 15, 2023
View reviewed changes
src/main/java/org/jenkinsci/maven/plugins/hp/util/Utils.java Show resolved Hide resolved
src/main/java/org/jenkinsci/maven/plugins/hpi/AbstractHpiMojo.java Outdated Show resolved Hide resolved
src/main/java/org/jenkinsci/maven/plugins/hpi/TagLibInterfaceGeneratorMojo.java
@@ -119,75 +119,75 @@ private void walk(File dir,JPackage pkg,String dirName) throws JClassAlreadyExis
c.annotate(TagLibraryUri.class).param("value",dirName);

JBinaryFile _gdsl = new JBinaryFile(c.name()+".gdsl");
PrintWriter gdsl = new PrintWriter(new BufferedWriter(new OutputStreamWriter(_gdsl.getDataStore(), StandardCharsets.UTF_8)));
Copy link
Member Author

Choose a reason for hiding this comment

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

review with whitespace ignored :)

@jtnord
remove direct guava usage
b4d46b4 
The only direct guava usage was a single function, so reimplented.

This prevents the downlaod of an extra dependency when running in a
clean environment like CI.
@jtnord
remove missed unused imports
ce934ab
@jtnord
sepcify the max heap for maven
59998d1 
this is picked up by the main maven process and all of the maven
processes used by the invoker plugin so there is no need to configure
maven invoker independantly
@jtnord jtnord marked this pull request as ready for review March 15, 2023 17:35
@jtnord jtnord merged commit ef62610 into jenkinsci:master Mar 15, 2023
@jtnord jtnord deleted the misc-cleanup branch March 15, 2023 17:36
@basil basil mentioned this pull request Apr 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@basil basil basil approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jtnord @basil