Skip to content

[core] Deprecate enumerated properties#1505

Merged
adangel merged 5 commits into
pmd:masterfrom
oowekyala:deprecate-enumerated-properties
Dec 8, 2018
Merged

[core] Deprecate enumerated properties#1505
adangel merged 5 commits into
pmd:masterfrom
oowekyala:deprecate-enumerated-properties

Conversation

@oowekyala

@oowekyala oowekyala commented Dec 6, 2018

Copy link
Copy Markdown
Member

Refs #1432

@oowekyala oowekyala added the is:deprecation The main focus is deprecating public APIs or rules, eg to make them internal, or removing them label Dec 6, 2018
@oowekyala oowekyala added this to the 6.10.0 milestone Dec 6, 2018
@oowekyala oowekyala mentioned this pull request Dec 6, 2018
Closed
36 tasks
@ghost

ghost commented Dec 6, 2018

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

Generated by 🚫 Danger

adangel
adangel reviewed Dec 7, 2018
View reviewed changes
Comment thread ...rc/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidUsingHardCodedIPRule.java
"checkAddressTypes", "Check for IP address types.", ADDRESSES_TO_CHECK,
Arrays.asList(IPV4, IPV6, IPV4_MAPPED_IPV6),
String.class, 2.0f);
private static final PropertyDescriptor<List<String>> CHECK_ADDRESS_TYPES_DESCRIPTOR =

@adangel adangel Dec 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I hope, it's ok to change this from public to private and change the type? I guess, nobody anyway directly used this property programmatically...

@adangel adangel Dec 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think, we should keep it public...

@adangel adangel self-assigned this Dec 8, 2018
@adangel

adangel commented Dec 8, 2018

Copy link
Copy Markdown
Member

I'll merge this now. There are two changes, we should think about:

These change the type of public properties from EnumeratedMultiProperty<String> to PropertyDescriptor<List<String>> - which is a API incompatible change.
We could say, that this is tolerable for properties, that are defined and used by only one rule.
Wdyt?

@adangel adangel merged commit b3fdf2f into pmd:master Dec 8, 2018
@oowekyala oowekyala deleted the deprecate-enumerated-properties branch December 8, 2018 14:15
@oowekyala

oowekyala commented Dec 8, 2018

Copy link
Copy Markdown
Member Author

Yeah I think for the moment, property descriptors for individual rules should be part of their implementation and so should be hidden. We could probably go so far as say that concrete rule classes are also not part of our published API ? We'd anyway need to mention that on the release notes though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adangel adangel adangel left review comments

Assignees

@adangel adangel

Labels

is:deprecation The main focus is deprecating public APIs or rules, eg to make them internal, or removing them

Projects

None yet

Milestone

6.10.0

Development

Successfully merging this pull request may close these issues.

2 participants

@oowekyala @adangel