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

Skip directory emptiness check for known tagtrees directories #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MyFedora
Copy link

Fixes #55

@novoid
Copy link
Owner

novoid commented Jul 31, 2023

This makes sense to me.

However, in order to maintain conistency, I'd rename ".filetag_tagtrees" to ".filetags_tagtrees".

Would you modify your pull-request accordingly? I could not find out how to make the change myself ...

@novoid novoid self-assigned this Jul 31, 2023
@MyFedora MyFedora force-pushed the fix/directory-emptiness-check branch 2 times, most recently from f593642 to 25b91c0 Compare August 1, 2023 16:50
@MyFedora
Copy link
Author

MyFedora commented Aug 1, 2023

Sure, I renamed the files accordingly. I also re-added the options.tagtrees_directory condition to ensure that existing users don't have to create a .filetags_tagtrees file manually, oops.

@MyFedora
Copy link
Author

MyFedora commented Aug 1, 2023

Allowing edits by maintainers
If checked, users with write access to novoid/filetags can add new commits to your fix/directory-emptiness-check branch.
You can always change this setting later.

I think you'd have to use my fork to make any changes yourself.

@MyFedora
Copy link
Author

MyFedora commented Aug 1, 2023

I'm writing unit tests for this change, but tests for this are rather flaky. generate_tagtrees always assumes that the directory parameter is the default directory in the test environment. assert_empty_tagfilter_directory skips the check for the default directory based on options.tagtrees_directory, but the tests never set any cli options. generate_tagtrees relies on assert_empty_tagfilter_directory, hence flaky tests.

I'll probably replace the options dependency with a check for the real default directory or smth, not quite sure yet.

@MyFedora MyFedora force-pushed the fix/directory-emptiness-check branch 4 times, most recently from 4f2f4ca to c789c84 Compare August 2, 2023 20:43
@MyFedora MyFedora force-pushed the fix/directory-emptiness-check branch from c789c84 to 29555f4 Compare August 2, 2023 20:49
@MyFedora
Copy link
Author

MyFedora commented Aug 3, 2023

I read #42 and wonder whether I should incorporate that into this PR. I'd be weird to create a .filetags-tagtrees file for tagtrees and tagfilters when there's an issue that expects separate files for each in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot reuse custom tagtrees directories
2 participants