Skip to content

Conversation

@shamlymhd
Copy link
Contributor

@shamlymhd shamlymhd commented Nov 22, 2022

Added a new constructor parameter errorOnEmptyFiles, default is true
If the parameter is set, log the empty files message as error. Otherwise, log it as info.

  • 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

Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

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

Please do not checkin unrelated files. You are also not using the correct coding style.

@shamlymhd shamlymhd requested a review from uhafner November 23, 2022 19:43
Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

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

We are almost there 😄

@uhafner uhafner changed the title info and error messages are configurable Make severity of scanning empty files configurable Nov 24, 2022
@uhafner uhafner added the enhancement Enhancement of existing functionality label Nov 24, 2022
@shamlymhd shamlymhd requested a review from uhafner November 24, 2022 10:58
@shamlymhd
Copy link
Contributor Author

I have two questions. if you can please respond

  1. I was only able to run the tests locally using "mvn test" but I couldn't run by right-clicking and selecting the test or selecting the test configs in IntelliJ idea. because will throw the "Error: could not open `{jenkins.addOpens}'"
  2. why build is failing here?

@uhafner
Copy link
Member

uhafner commented Nov 24, 2022

  1. {jenkins.addOpens}

I think you are using an outdated version of IntelliJ. (The removed parameter JavaDoc also was an IntelliJ bug that has been fixed recently). For details see: https://github.com/jenkinsci/plugin-pom/releases/tag/plugin-4.40

@shamlymhd
Copy link
Contributor Author

ok got it thank you and please review the PR

@uhafner
Copy link
Member

uhafner commented Nov 26, 2022

You need to fix the errors first.

@shamlymhd
Copy link
Contributor Author

Locally maven build is successful with java 17 and all the errors are gone after updating the IntelliJ to the 2022.2 version. I don't know the cause of this error here.

@uhafner
Copy link
Member

uhafner commented Nov 29, 2022

There is a compile error: https://github.com/jenkinsci/plugin-util-api-plugin/actions/runs/3570909394/jobs/6002310967

Can you merge with master and retry?

@uhafner
Copy link
Member

uhafner commented Nov 29, 2022

Maybe you did not commit all changes?

Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

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

You did merge with your master branch, but actually you needed to merge with my master branch 😄

@shamlymhd
Copy link
Contributor Author

Okay got it, Thank you.

@uhafner
Copy link
Member

uhafner commented Nov 30, 2022

I updated the following 2 PRs now with your changes to see if the consumers still work. When those builds are green, then I will merge and release this plugin:

Then we can continue with the last step for https://issues.jenkins.io/browse/JENKINS-69036.

I'll comment there what to do next.

@uhafner uhafner merged commit 9eec644 into jenkinsci:master Nov 30, 2022
@shamlymhd shamlymhd deleted the shamly-fix-i branch December 1, 2022 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants