Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removef "f" prefixes from field names #1191

Closed
wants to merge 5 commits into from
Closed

Removef "f" prefixes from field names #1191

wants to merge 5 commits into from

Conversation

alexpanov
Copy link

Fixed issue #1182, updated plugin versions

@@ -16,15 +16,15 @@
* @see ComparisonCompactor
*/
private static final int MAX_CONTEXT_LENGTH = 20;
private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

/*
* We have to use the f prefix until the next major release to ensure
* serialization compatibility.
* See https://github.com/junit-team/junit/issues/976
*/
Copy link
Member

Choose a reason for hiding this comment

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

We should remove these comments, too, shouldn't we?

Copy link
Author

Choose a reason for hiding this comment

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

@marcphilipp Yes. I have removed this comment and all the others that reference the same issue. I have also found another class with an f-prefixed field name and fixed it. Please take a look.

@kcooney
Copy link
Member

kcooney commented Aug 20, 2015

Although I appreciate the effort, and I'm very much not a fan of having classes with field names starting with f I don't think we should submit this change.

In an early RC for 4.12 we tried renaming the fields, and it broke at least one IDE due to the serialization incompatibilities. I suspect numerous IDEs and build tools would be broken by the serialization changes, and many projects would be broken that access private fields by reflection (yes, it's ugly, and we prefer people ask us to provide extension points, but many projects continue to access fields).

All combined, these changes would delay the migration to JUnit5.

The benefits of renaming the fields are quite small (minor readability improvements).

I recommend we not rename fields in classes unless we need to make other changes to the classes that cannot be made in a way that is backwards and forwards compatible with serialization. When that happens, we can more easily weigh the befits of adding the features vs the cost of breaking people.

The JUnit Lambda effort has as one of it's goals "Decouple test execution and reporting from test definition and provisioning". Hopefully that will involve writing a test execution API that isn't as fragile with respect to serialization. When that API exists and is adopted, it should be safer to rename these fields.

@kcooney kcooney changed the title Junit5 Removef "f" prefixes from field names Aug 31, 2015
@kcooney kcooney closed this Apr 22, 2016
@alexpanov alexpanov deleted the junit5 branch April 22, 2016 17:35
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.

None yet

3 participants