Add md4c, pymd4c and mdpo packages #102668
Conversation
There was a problem hiding this comment.
thanks for opening your first PR to nixpkgs.
Whats the value added to nixpkgs by adding this? the package seems to be a personal project with no release management (other than uploads to pypi). https://github.com/mondeja/mdpo
| @@ -2733,6 +2733,10 @@ | |||
| fingerprint = "67FE 98F2 8C44 CF22 1828 E12F D57E FA62 5C9A 925F"; | |||
| }]; | |||
| }; | |||
| euandreh = { | |||
| name = "EuAndreh"; | |||
| email = "eu@euandre.org"; | |||
There was a problem hiding this comment.
please add github (name) and githubId
pkgs/top-level/python-packages.nix
Outdated
| @@ -5183,6 +5185,8 @@ in { | |||
|
|
|||
| pymc3 = callPackage ../development/python-modules/pymc3 { }; | |||
|
|
|||
| pymd4c = callPackage ../development/python-modules/pymd4c { }; | |||
There was a problem hiding this comment.
| pymd4c = callPackage ../development/python-modules/pymd4c { }; | |
| pymd4c = callPackage ../development/python-modules/pymd4c { | |
| inherit (pkgs) pkg-config; | |
| }; |
|
Thanks for the review, @jonringer.
I've addressed your comments and rebased. I've only responded to the
comment on `githubId`.
Jonathan Ringer <notifications@github.com> writes:
> @@ -2733,6 +2733,10 @@
fingerprint = "67FE 98F2 8C44 CF22 1828 E12F D57E FA62 5C9A 925F";
}];
};
+ euandreh = {
+ name = "EuAndreh";
+ email = ***@***.***";
please add github (name) and githubId
I didn't want to tie being a package maintainer with my GitHub profile.
The file says this is an optional step [0], so I chose to not add it.
[0]: https://github.com/nixos/nixpkgs/blob/6e0320ebc12711f1f648f25e6cc497e49beb5004/maintainers/maintainer-list.nix#L8-L10
|
|
Jonathan Ringer <notifications@github.com> writes:
thanks for opening your first PR to nixpkgs.
Actually, this is my second. If you're inclined to review that too,
here's a shameless plug of it: #99544
|
|
Jonathan Ringer <notifications@github.com> writes:
Whats the value added to nixpkgs by adding this? the package seems to
be a personal project with no release management (other than uploads
to pypi). https://github.com/mondeja/mdpo
TBH, I didn't weighted this too much when packaging it. I haven't
encountered this type of discussion before, too.
Both PRs I've done are valuable tools for doing translations, and I find
them to be useful in a general sense, rather than personally only to me.
I have on my OS a few derivations packaged just for me and a few with
tweaked attributes, but both `sphinx-intl` and `mdpo` I don't think fit
the same category.
That is the value it adds to nixpkgs: translation tools I've packaged
for myself made available for other people.
What are you looking for other than the pypi releases?
|
release tags or branches. Something that says, "I want people to package this". |
Hello, I'm the maintainer of |
|
Álvaro Mondéjar <notifications@github.com> writes:
> release tags or branches. Something that says, "I want people to package this".
Hello, I'm the maintainer of `mdpo` package. The release process will
be updated to tags soon, but I've maintained using only PyPI because
the package have been in beta status, with breaking changes even
without major versions updates. I can report it here when will be
updated to tags.
Well, this definitively counts as "I want people to package this".
|
|
Done, as of v0.3.0, mdpo releases will be made through tags and it will strictly comply with semantic versioning. |
|
Well, this is actually very re-assuring |
The reason why I brought this up, is there's a costs in adding a package to nixpkgs (both in resources, and peoples time). The project should be "useful to many". This explanation should be added to the "motivation for this change" section of the PR template. |
|
Jonathan Ringer <notifications@github.com> writes:
This explanation should be added to the "motivation for this change"
section of the PR template.
ACK. Will do next time.
I've addressed the comments and updated to 0.3.0.
|
|
Now `mdpo` is also accessible as a top-level package:
```shell
nix-shell -I nixpkgs=. -p mdpo --run 'md2po --help'
```
|
|
Also thanks @mondeja for the responsiveness!
|
I'll try to find related topic, but they are needed as we had some issues with people changing their name and not being able to find them easily |
| euandreh = { | ||
| name = "EuAndreh"; | ||
| email = "eu@euandre.org"; | ||
| }; |
There was a problem hiding this comment.
| euandreh = { | |
| name = "EuAndreh"; | |
| email = "eu@euandre.org"; | |
| }; | |
| euandreh = { | |
| name = "EuAndreh"; | |
| email = "eu@euandre.org"; | |
| githubId = 2935736; | |
| github = "EuAndreh"; | |
| }; |
There was a problem hiding this comment.
@EuAndreh please take a look at jonringer's review comments.
|
EuAndreh <eu@euandre.org> writes:
Well, this is exactly what I mean. I'll first delete my GitHub account
than my email address. So the email is more important than the GitHub account.
I don't want it to block this contribution, though. If you're willing to
merge withouth the `githubId`, great. Otherwise I'll accept your
suggestion, too
|
|
Jonathan Ringer <notifications@github.com> writes:
I'll try to find related topic, but they are needed as we had some
issues with people changing their name and not being able to find them
easily
Well, this is exactly what I mean. I'll first delete my GitHub account
than my email address. So the email is more important than the GitHub account.
|
|
Sandro <notifications@github.com> writes:
At least ``pymd4c`` is python3.6 plus and fails to build on python27.
Can you change that and check the other packages for similar version
restrictions?
The change is just to use `python3Packages.buildPythonPackage` instead?
|
Otherwise it could mix and break the latest python with the version before that. |
|
I've added the `pythonOlder = "3.6"` restriction, updated mdpo to 3.5
and rebased with master.
|
|
EuAndreh <eu@euandre.org> writes:
I've added the `pythonOlder = "3.6"` restriction, updated mdpo to 3.5
and rebased with master.
It stopped working for some reason, let me investigate and fix it.
|
|
Sandro <notifications@github.com> writes:
Please add a comment why they are disabled and add a pythonImportsCheck.
Both added and rebased with master.
|
|
Result of 5 packages built:
python37Packages.pymd4c.log python38Packages.pymd4c.log Please add |
|
Result of 4 packages built:
python38Packages.pymd4c.log python37Packages.pymd4c.log Add |
|
On Mon, Dec 28, 2020 at 11:00:30AM -0800, Sandro wrote:
Add ``doCheck = false;`` and pythonImportsCheck.
Done, also fixed merge conflict with master.
|
| @@ -1,14 +1,22 @@ | |||
| { substituteAll, runtimeShell, jq, nix, lib }: | |||
There was a problem hiding this comment.
I think you did some mistake when rebasing. Please drop the changes to nixos-rebuild.
| euandreh = { | ||
| name = "EuAndreh"; | ||
| email = "eu@euandre.org"; | ||
| }; |
There was a problem hiding this comment.
@EuAndreh please take a look at jonringer's review comments.
|
Sandro <notifications@github.com> writes:
Add ``doCheck = false;`` and pythonImportsCheck.
Done.
|
|
Sandro <notifications@github.com> writes:
I think you did some mistake when rebasing. Please drop the changes to nixos-rebuild.
Oops, fixed. TY.
@EuAndreh please take a look at jonringer's review comments.
I did see his comment on adding the github id, but that being an
optional field [0], I'd rather not tie my contribution with my github
profile.
[0]: https://github.com/NixOS/nixpkgs/blob/57c198236a6c08f01286cfec6a5400383b561850/maintainers/maintainer-list.nix#L8-L10
|
You already did by opening this PR and if you ever rename your profile we just have a hard time finding your new name. It is not impossible and just makes the process harder. |
|
Sandro <notifications@github.com> writes:
> I'd rather not tie my contribution with my github profile.
You already did by opening this PR and if you ever rename your profile we just have a hard time finding your new name. It is not impossible and just makes the process harder.
I am totally fine if you do not want to add your real name, mail address gpg key or anything else but github name and id are already associated with this PR.
No hard time if I rename or delete my profile: the email will stay the same.
|
|
Closing stale pull request. Future interested parties may pick this up and merge in the future. |
Motivation for this change
Add
mdpopackage.Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)Execution of binary tested with: