Skip to content
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

chores #7

Merged
merged 4 commits into from
Apr 12, 2024
Merged

chores #7

merged 4 commits into from
Apr 12, 2024

Conversation

evlli
Copy link
Contributor

@evlli evlli commented Feb 13, 2024

closes: #5
closes: #6

@evlli evlli requested a review from a team as a code owner February 13, 2024 11:52
Copy link
Contributor

@jcgruenhage jcgruenhage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a few things in here I'd like to see changed:

  • The commit that adds the github workflows claims it's only doing the binary releases, but it's also doing containers and adds a flake.nix. The latter should be moved to a separate commit, and the former should be fixed by changing the commit message
  • Another commit changes the readme, adds a flake.lock, removes the shell.nix and removes a rustfmt config, as well as changes to the docker-compose.yml. This commit is especially in need of changes:
    • The nix changes should be moved to a separate commit, squashed together with the nix changes from the previous commit.
    • The rustfmt thing should move to the same commit that also applies the changes coming from that config change.
    • The documentation needs to be looked at again in general, that's still not really up to snuff, and should also move to a separate commit. I'd suggest looking at the rendered html coming out of that md doc.
    • That leaves this commit with only the docker-compose changes. The commit message should reflect that accordingly.

I hope this does not come off as too negative: I'm happy to see this happening and am looking forward to getting an adjusted version of this merged. Thanks for working on this!

@evlli evlli requested a review from jcgruenhage March 27, 2024 11:32
@jcgruenhage jcgruenhage self-assigned this Apr 4, 2024
@evlli evlli merged commit ccd714a into main Apr 12, 2024
2 checks passed
@evlli evlli deleted the evlli/chores branch April 12, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use nix flake update docs
2 participants