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

Streamline AppDirs paths #9676

Merged
merged 1 commit into from
Mar 18, 2023
Merged

Streamline AppDirs paths #9676

merged 1 commit into from
Mar 18, 2023

Conversation

koppor
Copy link
Member

@koppor koppor commented Mar 16, 2023

While discussing the backup path with @ThiloteE, I found, we are inconsistent.

Before:

├───100.0.0
│   └───backups
├───Logs
│   ├───100.0.0
│   ├───5.7--2022-08-05--73c111c
│   ├───5.8--2022-11-21--f7ea169
│   ├───5.8--2022-12-18--b7fae4b
│   ├───5.9--2023-01-04--bc3b9892f
│   ├───6.0--2023-01-01--b06ec3f
│   ├───6.0--2023-01-04--06771c8
│   └───unknown
├───lucene94
│   ├───-1446191503
│   ├───-1525049150
│   ├───-1981152844
│   ├───-2018577075
│   ├───-271657575
│   ├───-404722334
│   ├───-911001257
│   ├───1750725641
│   ├───1907816293
│   ├───647714185
│   ├───762261717
│   └───unsaved
└───ssl

After:

├───backups
├───logs
│   └───100.0.0
├───lucene94
│   ├───-1446191503
│   ├───-1981152844
│   ├───-2018577075
│   └───-404722334
└───ssl

Note that logs is spelled with lower-case l. This is consistent to ssl and lucene94. - We could discuss if we should introduce another level of hierarchy for lucene: lucene/94 to be consistent to logs.

Backups still work (checked by modifing a backup)

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@Siedlerchr
Copy link
Member

Needs to be tested with snaps.
For lucene im against moving the path. Will trigger reindex

@koppor
Copy link
Member Author

koppor commented Mar 17, 2023

For lucene im against moving the path. Will trigger reindex

I can add backwards compatibility. In case the old path exists, use that. If there is none, use the newer one.

@koppor
Copy link
Member Author

koppor commented Mar 17, 2023

For lucene im against moving the path. Will trigger reindex

I can add backwards compatibility. In case the old path exists, use that. If there is none, use the newer one.

Or even more simpler: If lucene94 is used, use the path as is. If a newer version is used, use the new structure.

@koppor
Copy link
Member Author

koppor commented Mar 17, 2023

Note the current PR does not touch the lucene path. -- We could do that when we modify the index at #8963?

@koppor
Copy link
Member Author

koppor commented Mar 17, 2023

Other note: jabref is always written in lower case latter. Before, there was lowercase at start, but at other places mixed case.

@calixtus
Copy link
Member

We are not honestly discussing backwards compatibility for temporary files? A refresher of the search index is not a bad thing. Of course, it takes some time to rebuild the index, if you install a new version of jabref every three to six months.

@Siedlerchr Siedlerchr merged commit b8497e9 into main Mar 18, 2023
@Siedlerchr Siedlerchr deleted the unify-paths branch March 18, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants