Fixing Good first issues#131
Merged
Merged
Conversation
`handle_md` does not need to be a separate function. It is only called once, and it doesn't offer any substantial encapsulation since it has preconditions that would require substantial work for any caller. Just combining the code that verifies that the markdown path is valid with the code that actually uses it makes it easier to tell that it is correct, not harder. While we are at it, using a regex is super overkill for this purpose, cgit is checked first in the `application` function so lines starting with `/cgit` will be directed to that handler before ever reaching the default case which calls this function, so we really only need to check whether or not the path ends with `.md` and there is `str.endswith` which does precisely that and does it faster.
Most routes are not patterns, we should just compare the path info with the exact expected string and avoid bloated regex invocations. While we are at it, match offers a more DRY solution to for checking rocket.path_info against many values.
We can remove some of the complexity about podman compose now that the important features for local development are merged upstream. We can convert all the sections to use lists instead of paragraph form to make them easier to read. Bulleted lists also make more sense than numbered lists for this purpose since there is some commentary and not just do this step and then this step.
canot -> cannot Fixes: 5222f97 ("orbit: hyperspace: add the ability clear a password hash")
If the user already has a limit imposed we should bail without changing it if we are trying to deny access. Passing `XATTR_CREATE` will ensure that fsetxattr enforces this.
the new `-u` option provides the ability to atomically update the limit that was the old behavior of `-d` before the previous commit.
This keeps version numbers in patch subjects.
flake8 pointed out that it was never used. No idea why I imported it originally... Fixes: ba2e68d ("denis: submit: refactor patchset processing logic to improve clarity")
This will make for easy lookups based on submission id.
theyoyojo
force-pushed
the
good_first_issue
branch
from
df06efe to
231db71
Compare
theyoyojo
reviewed
Jul 17, 2024
theyoyojo
reviewed
Jul 17, 2024
theyoyojo
reviewed
Jul 17, 2024
theyoyojo
approved these changes
Jul 17, 2024
theyoyojo
left a comment
theyoyojo
left a comment
Contributor
There was a problem hiding this comment.
besides comments on the readme which will get some more attention soon anyway and is already way better, lgtm
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #108
Fixes #114
Fixes #120
Fixes #122
Fixes #128
Fixes #130