Skip to content

Remove nix - #291

Open
qqshfox wants to merge 2 commits into
masterfrom
remove-nix
Open

Remove nix#291
qqshfox wants to merge 2 commits into
masterfrom
remove-nix

Conversation

@qqshfox

@qqshfox qqshfox commented Feb 23, 2021

Copy link
Copy Markdown
Contributor
  1. Introduce Poetry to lock dependency versions
  2. Remove nix stuff
  3. Add plain-Dockerfile

Resolves #288

@qqshfox
qqshfox force-pushed the remove-nix branch 7 times, most recently from 5018477 to 8dab6c7 Compare March 1, 2021 15:04
@brettdh

brettdh commented Mar 3, 2021

Copy link
Copy Markdown

I'm not a maintainer, but I suspect this would be more likely to be accepted if it adds a plain Dockerfile rather without removing the Nix setup. This could include removing the dockerize stuff from the Nix setup, but not removing the Nix setup altogether. As was discussed on #288, there are still users who prefer to deploy marge-bot via Nix.

@nejch nejch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd also say points 2 and 5 could be split into their own PR that gets merged first, to make the purpose of this PR clearer (basically cherry-pick fdd7721 but without the dependency upgrades).

And just my opinion but at this point GitHub Actions are probably nicer for quick CI tests than Travis ;) At least in my experience as of late 2020 Travis quality and job queues have deteriorated. Maybe not for deployment yet, as you have secrets stored already from what I see.

@qqshfox

qqshfox commented May 5, 2021

Copy link
Copy Markdown
Contributor Author

@nejch thanks for the feedback. I've submitted a new MR to upgrade the dependencies and fix lint warnings introduced by the latest linters. However other parts, e.g. testing with more python versions, plain-Dockerfile, are all heavily based on the removal of nix. I'm not sure if it's viable to rewrite them along with the nix stuff.

@nejch

nejch commented May 6, 2021

Copy link
Copy Markdown
Contributor

@nejch thanks for the feedback. I've submitted a new MR to upgrade the dependencies and fix lint warnings introduced by the latest linters. However other parts, e.g. testing with more python versions, plain-Dockerfile, are all heavily based on the removal of nix. I'm not sure if it's viable to rewrite them along with the nix stuff.

Yeah that's basically what I had in mind for splitting up the linting issues, thanks :)

@qqshfox qqshfox added this to the 0.11.0 milestone Jun 2, 2021
@qqshfox
qqshfox force-pushed the remove-nix branch 3 times, most recently from 9c10ea8 to 0452c4c Compare June 3, 2021 06:43
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.

Is Nix worth the complexity?

3 participants