Misc fixes #188
Merged
Misc fixes #188
Conversation
This file contains 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
To be consistent with all other volumes, the fact that the git container has write access to its volume should be made explicit by adding an explicit `read_only: false`.
we can avoid needing to refer to containers by their fully qualified names by using podman-compose which will map the name of the service to the name of the underlying container for us. This makes the tests not dependent on the flaky and environment specific scheme that the compose provider uses for assigning container names and allows the tests to be able to be used in more situations.
Many of these were incorrect and referred to /var/lib/email as a single volume despite it having been split up into more than one a while ago and all of them were useless because the path exists in the image whether or not something is mounted over it. They just add noise to the Containerfiles without providing an upside so I say chuck 'em.
In order for comparisons between the values we create by shifting multiple bytes into a single uint32_t to work as expected, the first character in a multicharacter character literal needs to correspond to the byte with the largest place value so that the constants when read from left to right will match the first character with the character that has been shifted three times and the last character with the character that hasn't been shifted at all.
The journal will be managed by denis in the future, so disentangling the journal code from the pop code will be helpful as a first step.
This header is really only used for the one shared C type, the struct that represents an email and it is unlikely that other types will be added, so renaming it makes it clearer what the header is for.
Since denis will be maintaining the journal in the future it makes sense for it to control the whole lifecycle of the journal file.
Switch the test scripts over to use denis so that building the journal programs can be completely removed from pop and it can be switched to have read only access to the journal volume.
By forcing the shell to exit we kill pid 1 of the container which tears down all other processes immediately and ungracefully with sigkill. By simply changing the code we execute in the trap instruction to kill the last background job with sigint, we can bring python down gracefully which will make `wait` return and then exit the shell.
calling .append will mutate the array in the parent scope because args is the same object unless a deepcopy is made. This was happening to work because we only called the function twice in the order where initially it wasn't being appended.
The path did not point to where the hooks actually were so they were not running. This wasn't an issue for pushing data and seeing it appear on cgit because it does not rely on `git update-server-info` but a clone of the repo via the git container http server would not reflect changes made by pushing new content in after building the image.
when creating the peer review a latent reference to a `comments` field that did not make it into the final version of the code that was merged lingered in the code. It was harmless because a value of `None` was being passed which peewee ignored, but it is dead/incorrect code.
theyoyojo
approved these changes
Oct 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Random improvements discovered / inspired while working on email automation: