-
Notifications
You must be signed in to change notification settings - Fork 251
lib/getdate.y: Restrict the date formats that we support #1238
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
Conversation
This is much clearer to my eyes. To the extent that a goal here is to separate concerns while keeping a clean minimal commit history for review, I do see two fairly straightforward opportunities for squashes:
I also note that the many global variables referred to by this comment are gradually whittled away, rendering it somewhat outdated by the end of this PR. As I mentioned on #1217 (comment), this PR should ensure relevant documentation is updated in order to be complete. |
Sounds reasonable. I've squashed them.
I never understood that comment. I'll keep it, just because I don't understand it, so I'll remove it in the commit that gets rid of the weird yacc stuff, in the other PR.
Thanks! I've cherry-picked the documentation changes. |
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've added two minor comments.
In addition, since we are already refactoring the code I think we should improve the existing documentation. I've asked our internal documentation team whether they can help us with this task.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Our caller, strtoday(), already handles a raw number. Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar do you mind removing Dominika's commits? I think the documentation is not ready yet and we still need to work on improving it before merging. |
Those two commits are good IMO. I think they're useful per se, and maybe we can iterate over them later. I didn't like the rest of commits as they are, but these two seem a net improvement. But if you prefer to have more review about them, I can drop them for now. Just let me know what you prefer. |
Yeah, I would prefer to work on them in their own PR and merge all the documentation together. |
Okay, I've dropped them. |
container-build fedora is failing, but I don't understand why. @ikerexxe , do you know? :| |
Newly released fedora 42 had a missing dependency. Fix is available at #1252 |
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 forgot to come back and approve after I fixed the CI failure. LGTM! Thanks for the patches.
This PR removes features of get_date() that we don't really need or want.
It is in preparation of a future replacement of this code by a C implementation.
Cc: @zeha , @timparenti
Revisions:
v1b
Range-diff:
Interdiff:
v2
v3
strtoday()
.v4
v4b
v4c
v5