Skip to content

[java] New Performance Rule UseIOStreamsWithApacheCommonsFileItem#2574

Merged
adangel merged 7 commits into
pmd:masterfrom
adangel:perf-UseIOStreamsWithApacheCommonsFileItem
Jun 26, 2020
Merged

[java] New Performance Rule UseIOStreamsWithApacheCommonsFileItem#2574
adangel merged 7 commits into
pmd:masterfrom
adangel:perf-UseIOStreamsWithApacheCommonsFileItem

Conversation

@adangel

@adangel adangel commented Jun 13, 2020

Copy link
Copy Markdown
Member

Describe the PR

This is the rule "UseIOStreamsWithApacheCommonsFileItem" split from original PR #1932 .

Discussion points:

  • The name: Maybe we should directly use "UseGetInputStreamWithApacheComonsFileItem" - since that is the only suggested solution?
  • This rule applies to a specific framework (apache commons). Just to keep in mind. We have not many rules, that are that specific.
  • Priority is 2 - we have a mix of priorities between 1 and 3. Not sure, if we need to care about the priority.

Related issues

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by travis)
  • Added (in-code) documentation (if needed)

@adangel adangel added the a:new-rule Proposal to add a new built-in rule label Jun 13, 2020
@ghost

ghost commented Jun 13, 2020 •
edited by ghost
Loading

Copy link
Copy Markdown
1 Message
📖 This changeset introduces 7 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.
Full report
✅ This changeset introduces 7 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.
Full report

Generated by 🚫 Danger

@adangel adangel mentioned this pull request Jun 14, 2020
Closed
@adangel adangel added this to the 6.25.0 milestone Jun 18, 2020
@oowekyala oowekyala self-assigned this Jun 21, 2020
@oowekyala

oowekyala commented Jun 21, 2020

Copy link
Copy Markdown
Member

Priority is 2 - we have a mix of priorities between 1 and 3. Not sure, if we need to care about the priority.

Maybe these should all be set to 3 at some point? It may be surprising otherwise.

@adangel

adangel commented Jun 22, 2020

Copy link
Copy Markdown
Member Author

Priority is 2 - we have a mix of priorities between 1 and 3. Not sure, if we need to care about the priority.

Maybe these should all be set to 3 at some point? It may be surprising otherwise.

I've changed the prio of the two new rules to 3 now. Let's keep the others as they are - it might be more surprising, if we change them now...

@adangel adangel assigned adangel and unassigned oowekyala Jun 26, 2020
@adangel adangel merged commit 3831a85 into pmd:master Jun 26, 2020
@adangel adangel deleted the perf-UseIOStreamsWithApacheCommonsFileItem branch June 26, 2020 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@adangel adangel

Labels

a:new-rule Proposal to add a new built-in rule

Projects

None yet

Milestone

6.25.0

Development

Successfully merging this pull request may close these issues.

3 participants

@adangel @oowekyala @jborgers