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

Resolve more request changes and warnings #5

Merged
merged 15 commits into from
Mar 6, 2024

Conversation

karlsb
Copy link

@karlsb karlsb commented Mar 5, 2024

Requested Changes:

  • Reverse if else condition in RemoveLinksToNotExistentFiles#cleanup
  • Store bibFolder.resolve("test.bib") in variable in RemoveLinksToNotExistentFilesTest#setup
  • Rename variable fileFolder to originalFileFolder in RemoveLinksToNotExistentFilesTest#setup
  • Remove comment in RemoveLinksToNotExistentFilesTest#setup
  • Rename variable Path defaultFileFolder to Path newFileFolder in RemoveLinksToNotExistentFilesTest#setup
  • Indent .withField statements in RemoveLinksToNotExistentFilesTest#deleteFileInEntryWithMultipleFileLinks
  • Changed LinkedFile constructor calls to use third parameter in RemoveLinksToNotExistentFilesTest
  • Replaced Arrays.asList() with List.of() in RemoveLinksToNotExistentFilesTest
  • Assert full lists instead of elements in RemoveLinksToNotExistentFilesTest
  • Use java.nio for file deletion in RemoveLinksToNotExistentFilesTest
  • Formatting comments correctly in RemoveLinksToNotExistentFilesTest#deleteLinkedFile
  • Change test name deleteFileInMultipleLinkedEntry to deleteFileInEntryWithMultipleFileLinks

Other Changes:

  • Changed class fields Path newFileFolder FilePreferences filePreferences and BibDatabaseContext databaseContext into local variables in RemoveLinksToNotExistentFilesTest
  • Removed unused variables LOGGER and fileHandler in RemoveLinksToNotExistentFiles

@burcukilic burcukilic merged commit 9d43c8d into main Mar 6, 2024
burcukilic added a commit that referenced this pull request Mar 6, 2024
* Adds feature CleanupJob for removing non-existent files (#2)

* feat: RemoveLinksToNotExistentFiles.java

* feat: added option to cleanup entries

* feat: improved cleanup entry dialogue

* Test: implements test for linked files which exist

Implementation of a test which calls the cleanup functionality of
RemoveLinksToNotExistentFiles with a entry that is linked to an existing
file. The test makes sure that no changes are made since the linked
file exists.

* Refactor: refactored test to match the standard of the project

Removed the JavaDoc comment from the test, since other tests in the
project don't have JavaDoc:s. Also removed test from the name of the
test function, as it seems that the standard way of naming tests
is to not have test in the testfunctions name.

* Refactor: removed system.out.print()

Removed an print from the tests.

* fix: non english character deleted

* Docs: Added information about changes

Added information about the changes made for solving issue#10929
to the CHANGES.md file.

---------

Co-authored-by: burcukilic <[email protected]>

* Fix: Removes unused imports and solves other errors

Removes unused imports and solves the error of language keys
that were not in the english laguage file.

* Fix: Removes empty line and solves other issues

Removes an empty line where 2 empty lines appear in after each
other. Also adds brackets to URL and removes an key which is
not in the english language file and was missed in the last commit.

* Docs: Removes technicality from changelog

Removes technical details for the change made in changelog and also
follows the suggestion for removing the space and brackets between
the link and the issue.

* test: enhanced RemoveLinksToNotExistentFilesTest (#3)

* test: enhanced RemoveLinksToNotExistentFilesTest

* test: add real content

* test: change structure

* test: added expectedChanges

* test: added a third test to RemoveLinksToNotExistentFilesTest

* refactor: removed unnecessary toString

* style: removed a whitespace

* refactor: checkstyle formatting.

formatted files according to checkstyle plugin.

---------

Co-authored-by: karlsb <[email protected]>

* Resolve request changes (#4)

* test: enhanced RemoveLinksToNotExistentFilesTest

* test: add real content

* test: change structure

* test: added expectedChanges

* test: added a third test to RemoveLinksToNotExistentFilesTest

* refactor: removed unnecessary toString

* style: removed a whitespace

* refactor: checkstyle formatting.

formatted files according to checkstyle plugin.

* refactor: replaced .get(0) with .getFirst() for first accessing element of List.

Changed was required from ./gradlew rewriteDryRun

---------

Co-authored-by: burcukilic <[email protected]>

* Resolve more request changes and warnings (#5)

* Reverse the condition and the content in RemoveLinksToNotExistentFiles

Resolves comment
 "Reverse the condition and the content.

Reason: You have BOTH (true and false) cases covered. The true case should normally come first."

* Fix: Store bibFolder.resolve("test.bib") in a variable

* Refactor: Rename variable fileFolder to originalFileFolder and remove comment.

* Refactor: Rename defaultFileFolder to newFileFolder

* Refactor: Fix indentation in RemoveLinksToNotExistentFilesTest

* Refactor: Replaced Arrays.asList() with List.of()

* Refactor: Replaced Arrays.asList() with List.of()

* Refactor: Change to use java.nio for file deletion.

* Refactor: Move comment to line above.

* Refactor: Added "PDF" as third argument to LinkedFile for OnlineLink files

* Refactor: Removed unused variables in RemoveLinksToNotExistentFiles

* Refactor: Throw IOException in RemoveLinksToNotExistentFilesTest functions

* Refactor: Tests assert using List.of() in RemoveLinksToNotExistentFilesTest

* Refactor: Corrected Arguments for LinkedFile in RemoveLinksToNotExistentFilesTest

* Refactor: Class fields into local variables

This refactor fixed 3 warnings.

* docs: updated docs to remove merge conflict

---------

Co-authored-by: Linus Wallin <[email protected]>
Co-authored-by: burcukilic <[email protected]>
Co-authored-by: Linus Wallin <[email protected]>
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.

2 participants