Skip to content

md2gemini: init at 1.8.1#117917

Merged
SuperSandro2000 merged 4 commits intoNixOS:stagingfrom
KAction:md2gemini
Apr 8, 2021
Merged

md2gemini: init at 1.8.1#117917
SuperSandro2000 merged 4 commits intoNixOS:stagingfrom
KAction:md2gemini

Conversation

@KAction
Copy link
Contributor

@KAction KAction commented Mar 29, 2021

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@KAction KAction requested review from FRidh and jonringer as code owners March 29, 2021 00:07
@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Mar 29, 2021
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be clearer if we converted the current default into a generic nix and created a 0.8.nix and a 2.nix file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did it same way as postgresql do.

Comment on lines 6 to 8
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
propagatedBuildInputs = [ mistune_2 cjkwrap wcwidth ];
checkInputs = [ pytest ];

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{ lib, buildPythonPackage, fetchPypi, mistune_2, cjkwrap, wcwidth, pytest }:
{ lib, buildPythonPackage, fetchPypi, mistune_2, cjkwrap, wcwidth, pytestCheckHook }:

Copy link
Member

Choose a reason for hiding this comment

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

That causes an eval error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That does not belong here, removed.

@KAction
Copy link
Contributor Author

KAction commented Apr 2, 2021

Looks like I applied all suggestions.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Apr 2, 2021
@SuperSandro2000
Copy link
Member

@ofborg eval

@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. label Apr 3, 2021
@KAction
Copy link
Contributor Author

KAction commented Apr 3, 2021

Okay, now @ofborg is happy.

@KAction KAction requested a review from SuperSandro2000 April 3, 2021 19:41
Copy link
Member

Choose a reason for hiding this comment

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

Missing maintainer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was this way before.

@KAction
Copy link
Contributor Author

KAction commented Apr 6, 2021

Okay, I added pythonImportsCheck to mistune, but I'd rather not assume maintainership of every indirect dependency I touch. Wh

@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Apr 6, 2021
@SuperSandro2000
Copy link
Member

Okay, I added pythonImportsCheck to mistune, but I'd rather not assume maintainership of every indirect dependency I touch. Wh

The problem is that if your dependencies are not maintained they are potentially in a worse state but it seems like mistune is used in a lot more packages.

I think adding the imports check to mistune triggered the mass rebuild. We really want to have one if there are not tests because otherwise the package can silently break. I think the best solution would be to merge this PR into staging because if we split the change out we force a merge conflict which someone would need to resolve. Let me know if thats a problem for you.

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

other than that the ordering could be a bit more standard LGTM if rebased to staging

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

other than that the ordering could be a bit more standard LGTM if rebased to staging

@KAction
Copy link
Contributor Author

KAction commented Apr 7, 2021

Yes, sure. Let's merge it into staging.

@SuperSandro2000
Copy link
Member

/rebase staging

@github-actions github-actions bot changed the base branch from master to staging April 8, 2021 00:09
@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2021

Rebased, please reopen the pull request to restart CI

@github-actions github-actions bot closed this Apr 8, 2021
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 8, 2021
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 8, 2021
@SuperSandro2000 SuperSandro2000 merged commit 97490e6 into NixOS:staging Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants