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

Minor code quality work #9759

Merged
merged 2 commits into from
Apr 15, 2023
Merged

Minor code quality work #9759

merged 2 commits into from
Apr 15, 2023

Conversation

koppor
Copy link
Member

@koppor koppor commented Apr 13, 2023

This ports non-http-server related changes from #9748 to a smaller PR

  • Documentation / code style
    • Refined BibEntry class (JavaDoc, builder)
    • Comments to InternalField
    • move StyleTester to "test" module - package ...testutils/interactive...
  • Made class "JabRefItemDataProvider" more visible
  • Encoding of a .bib file can now be asked for externally
  • Fixes typo in NetworkTabViewModel

Compulsory checks

@koppor koppor added the type: code-quality Issues related to code or architecture decisions label Apr 13, 2023
@Siedlerchr
Copy link
Member

org.opentest4j.AssertionFailedError: The following classes are not allowed to depend on org.jabref.preferences.JabRefPreferences ==> expected: <[]> but was: <[src/test/java/org/jabref/testutils/interactive/styletester/StyleTesterMain.java]>

@Siedlerchr
Copy link
Member

Because you moved it to a folder under test, the architecture test says it's a violation because you depend on prefs. I tried adding an exception but does not work

calixtus
calixtus previously approved these changes Apr 13, 2023
@Siedlerchr
Copy link
Member

Still failing unit architecture test

@Siedlerchr Siedlerchr added the status: changes required Pull requests that are not yet complete label Apr 13, 2023
@koppor
Copy link
Member Author

koppor commented Apr 14, 2023

In d8f5b06 (#9759), I completely rewrite the checks for the test archiecture.

  1. Use ArchUnit instead of self-crafted testing
  2. Introduce new tests for test class naming consistency

Because of the second test, many test classes have been renamed to end with Test. We still have around ten exceptions, which are all helper classes.

@koppor koppor removed the status: changes required Pull requests that are not yet complete label Apr 14, 2023
@Siedlerchr Siedlerchr merged commit 9d5f7fc into JabRef:main Apr 15, 2023
@koppor koppor deleted the add-comments branch April 15, 2023 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: code-quality Issues related to code or architecture decisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants