Skip to content

[core] Deprecate regex property#1487

Merged
adangel merged 3 commits into
pmd:masterfrom
oowekyala:deprecate-regexProperty
Nov 30, 2018
Merged

[core] Deprecate regex property#1487
adangel merged 3 commits into
pmd:masterfrom
oowekyala:deprecate-regexProperty

Conversation

@oowekyala

@oowekyala oowekyala commented Nov 29, 2018

Copy link
Copy Markdown
Member

Refs #1432

I do this before StringProperty so that when updating the usages of StringProperty, we may convert some to a regex property where appropriate

@oowekyala oowekyala added this to the 6.10.0 milestone Nov 29, 2018
@ghost

ghost commented Nov 29, 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 approved these changes Nov 30, 2018
View reviewed changes

@adangel adangel left a comment

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.

Looks good.
I'd still add at least basic tests to PropertyDescriptorTest - so that we don't hit any surprises in 6.x with the new PropertyFactory...

@adangel adangel self-assigned this Nov 30, 2018
@oowekyala

oowekyala commented Nov 30, 2018

Copy link
Copy Markdown
Member Author

Good idea. I'll have some time for that next week

@oowekyala oowekyala added the is:WIP For PRs that are not fully ready, or issues that are actively being tackled label Nov 30, 2018
@adangel adangel merged commit 73cf377 into pmd:master Nov 30, 2018
@adangel

adangel commented Nov 30, 2018

Copy link
Copy Markdown
Member

I've merged this one now and added some minimal tests in PropertyDescriptorTest.
We probably should also test the conversion from/to String, which I didn't do yet.

@adangel adangel removed the is:WIP For PRs that are not fully ready, or issues that are actively being tackled label Nov 30, 2018
@oowekyala oowekyala deleted the deprecate-regexProperty branch December 1, 2018 05:06
@oowekyala

oowekyala commented Dec 1, 2018

Copy link
Copy Markdown
Member Author

Thanks for taking care of it!

adangel added a commit that referenced this pull request Dec 1, 2018
@adangel
Add more tests in PropertyDescriptorTest
66315c6 
Refs #1487
@adangel

adangel commented Dec 1, 2018

Copy link
Copy Markdown
Member

I've just now extended the test cases, so that we test the conversion from String via valueFrom - this is the current implementation that is used when we parse a ruleset. So having this in the tests should ensure, that we still can parse properties :)

@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 3, 2018
@oowekyala oowekyala mentioned this pull request Dec 3, 2018
Closed
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adangel adangel adangel approved these changes

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