-
Notifications
You must be signed in to change notification settings - Fork 169
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
Added support for -a, --force-album argument #491
Conversation
Test failures and permission denied when I run tox locally:
But when I checkout main the tests also fail albeit without the permission errors or verbosity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, seems a useful feature. Some comments below.
src/sigal/gallery.py
Outdated
@@ -823,7 +824,7 @@ def get_albums(self, path): | |||
for subname, album in self.get_albums(subdir): | |||
yield subname, self.albums[subdir] | |||
|
|||
def build(self, force=False): | |||
def build(self, force=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to change the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I was trying to avoid the implying it could just be a bool
src/sigal/gallery.py
Outdated
if fnmatch(a.path, f): | ||
return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if fnmatch(a.path, f): | |
return True | |
return fnmatch(a.path, f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that works because a False would be an early return
src/sigal/gallery.py
Outdated
elif a.name == f: | ||
return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
elif a.name == f: | |
return True | |
else: | |
return a.name == f |
Permission denied: this depends on your system, it seems the process cannot write a file (in |
Also please rebase on main, I fixed an issue with tox for the CI jobs. |
d82b376
to
6ade36c
Compare
Added tests and addressed comments. After fixing all of the permissions, tox finally runs, but it it won't get past the 2nd dot in the cli_tests after running for an hour. Pytest shows the same behavior and even on main without my changes, it looks like some tests fail, but without it actually completing, I can't tell what the errors are. |
Sorry for the delay @KaibutsuX, I forgot about this...
|
Like --force, but can specify multiple specific album names or wildcard patterns to apply to entire album paths like: -a Pics/* -a *Wedding* -a 'Landscape Photos'
d655a51
to
81c0d81
Compare
Codecov Report
@@ Coverage Diff @@
## main #491 +/- ##
==========================================
+ Coverage 87.53% 87.60% +0.07%
==========================================
Files 24 24
Lines 2053 2065 +12
==========================================
+ Hits 1797 1809 +12
Misses 256 256
|
FYI, I dont know what is going on with the local testing env. When I run |
Maybe an issue with opening an HTTP port ? Does it work with |
Like --force, but can specify multiple specific album names or wildcard patterns to apply to entire album paths like:
-a Pics/*
-a Wedding
-a 'Landscape Photos'