Skip to content

refactor: remove usage of NumberUtils#80

Merged
basil merged 3 commits intojenkinsci:masterfrom
BobDu:remove-number-utils
Jul 31, 2025
Merged

refactor: remove usage of NumberUtils#80
basil merged 3 commits intojenkinsci:masterfrom
BobDu:remove-number-utils

Conversation

@BobDu
Copy link
Copy Markdown
Member

@BobDu BobDu commented Jul 31, 2025

ref: #41 (comment)

I created a test case and run the unit test on the pre-refactored and post-refactored versions, and both passed successfully.

Testing done

Submitter checklist

  • 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 demonstrate the feature works or the issue is fixed

@basil basil changed the title refactor: remove usage of NumberUtils refactor: remove usage of NumberUtils Jul 31, 2025
Copy link
Copy Markdown
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.

Thanks Bob. Ideally for changes that are slightly more risky like this, we'd do testing with the core test suite, PCT, and ATH. I'll run these tests after merging this and update the links here so you can see how I did it. If you (or someone else) ever does prepare a core PR to remove Commons Lang 2, we'd definitely want those tests run for that.

@basil basil merged commit f7dfc90 into jenkinsci:master Jul 31, 2025
12 of 14 checks passed
@basil
Copy link
Copy Markdown
Member

basil commented Jul 31, 2025

@BobDu I tested this PR (and all your other changes) here:

Those 3 PRs show how to run a comprehensive set of tests on a given change.

I found only one small test-only regression in the Pipeline: Nodes and Processes plugin. I opened a PR to fix that here:

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.

2 participants