-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add skip
status to ignore selected posts
#3305
Conversation
c932252
to
34c921b
Compare
Because I have many drafts, this also has the side effect of speeding up my publish builds by 80%. That's nice. |
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.
Nice work, @GiovanH. I am not sure what the best name for this status should be, but I think I would prefer skip
over disabled
. (I also considered ignore
, but I think skip
is clearer.)
Also, since there are a number of new commits to main
, this PR should be rebased to pick up those changes.
I realize this is still a draft PoC, but in the interest of clear communication, I think we should also add tests for this new code before merging. After rebasing on main
, you should be able to run invoke coverage
on main
and then again on this PR branch in order to compare the output and see which new lines need test coverage.
@GiovanH: Two quick things…
|
I just wanted to get a thumbs up from you on the feature first. I'll go ahead and write the tests/docs now. |
Ciao Giovanni. Just checking in… Any chance you could get this PR ready this weekend? |
Thanks for the reminder, I'll see what I can do |
34c921b
to
dbc85aa
Compare
I am continuing to have incredible difficulties with the build environment (as I always seem to with this project) so it might take a bit longer. Renamed and rebased. Coverage looks good. Because the nature of this feature is that articles marked "skip" are to be ignored entirely, I think the test updates may consist of just adding a skipped article and expecting tests to remain the same, like so: |
dbc85aa
to
87dc3fe
Compare
I think I might actually not like |
87dc3fe
to
e2c19e0
Compare
703f105
to
221973a
Compare
Nice work, @GiovanH. Really appreciate your efforts.
Once we started overloading beyond Since "draft" is already taken, I say we come up with another status that implies unfinished, which Pelican then handles by ignoring it. Some potential candidates could be:
Personally, I like the first three options more than I like the last three. |
@GiovanH: On a side note, you previously said:
Which specific difficulties have you had? Which specific area(s) of the documentation about setting up the development environment are in need of improvement? |
Of these, I think only In defense of skipped and disabled, I'd point to "hidden", which is also past-tense descriptive. Pick whichever one you'd prefer out of stub, skip, skipped, or disabled. For the time being, I'll leave in
I think the documentation is fine, it's just that Most scripts that write files also use the "system default", and I have to mess with autocrlf or dos2unix to get the expected output. |
2ce3327
to
cff9af1
Compare
Another status option I think fits this use case: |
I don't hate it, but I think it has the same problem of not clearly communicating the behavior. Drafts are incomplete articles too, but the point of the new status is to behave differently. It's up to you though. |
bba4c1f
to
f70ff5b
Compare
Are you still looking to get this in soon? |
Hey @GiovanH. Thanks for your patience. I had intended on shipping a Pelican release last month, but my schedule took a hectic turn. My hope is to be able to dedicate some time to this in mid-August. I am still conflicted about what status moniker to use for this, but given the discussion up to this point, I’m inclined to go with: |
2405a5a
to
ce09855
Compare
ce09855
to
f6d2b59
Compare
@justinmayer Should be ready. |
Co-authored-by: Paolo Melchiorre <[email protected]>
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.
Many thanks to @GiovanH for the excellent work on (and patience with!) the implementation of this feature. Thanks also to @pauloxnet for reviewing! ✨
skip
status to ignore selected posts
Pull Request Checklist
Resolves: #3304
Proof of concept, let me know if you like this direction and I'll write test cases and documentation.