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.

Sign up for GitHub

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

Jump to bottom

DocBook schemas: refactor to pkgs/by-name/, add 5.1 and 5.2 #351706

Open
wants to merge 5 commits into
base: staging
Choose a base branch
from
Open

DocBook schemas: refactor to pkgs/by-name/, add 5.1 and 5.2 #351706

wants to merge 5 commits into from

Conversation

rc-zb
Copy link
Contributor

@rc-zb rc-zb commented Oct 27, 2024 •
edited
Loading

Refactor DocBook schemas (including the normative versions and the EBNF module) into pkgs/by-name/.
Add DocBook 5.1 and 5.2.
And add myself to the maintainer lists.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@rc-zb rc-zb force-pushed the refactor-and-update-docbook branch from 4686437 to 6df97c9 Compare October 27, 2024 17:06
@rc-zb rc-zb force-pushed the refactor-and-update-docbook branch from 6df97c9 to 3df7dae Compare October 28, 2024 02:30
@rc-zb rc-zb changed the base branch from master to staging October 28, 2024 03:00
@rc-zb rc-zb force-pushed the refactor-and-update-docbook branch 7 times, most recently from d848e76 to 12114e7 Compare October 29, 2024 03:10
@ofborg ofborg bot requested a review from edolstra October 29, 2024 06:25
@rc-zb rc-zb changed the title Refactor and update DocBook DocBook schemas: refactor to pkgs/by-name/, add 5.1 and 5.2 Oct 29, 2024
@Aleksanaa
Copy link
Member

@AndersonTorres has been working on this. See #351040

@rc-zb rc-zb force-pushed the refactor-and-update-docbook branch 2 times, most recently from 8bdcc41 to f4175b7 Compare October 29, 2024 08:12
@rc-zb rc-zb marked this pull request as ready for review October 29, 2024 08:20
@rc-zb
Copy link
Contributor Author

rc-zb commented Oct 29, 2024

@AndersonTorres has been working on this. See #351040

I think my refactoring is nearer to completion until now.

And the naming conventions differ: the docbook5 package refers to docbook_5_0 here and docbook-5 there; docbook_xml_dtd_45 to docbook_4_5 here and docbook-xml-dtd-4_5 there. I don't know which ones are canonical in Nixpkgs, though. Are the version differences analogue to racket and racket_7_9, or julia_110 and julia_19, or firefox and firefox-devedition?

@AndersonTorres AndersonTorres mentioned this pull request Oct 30, 2024
Closed
4 tasks
@AndersonTorres
Copy link
Member

If someone is interested in keeping this, feel free.
Less work to me.

AndersonTorres
AndersonTorres reviewed Oct 30, 2024
View reviewed changes
Copy link
Member

@AndersonTorres AndersonTorres left a comment

Choose a reason for hiding this comment

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

The comments are more suggestive than normative.

pkgs/by-name/do/docbook/generic-sgml-dtd.nix Outdated Show resolved Hide resolved
pkgs/by-name/do/docbook/generic-xml-dtd.nix Outdated Show resolved Hide resolved
pkgs/by-name/do/docbook/generic-sgml-dtd.nix Outdated Show resolved Hide resolved
AndersonTorres
AndersonTorres reviewed Oct 30, 2024
View reviewed changes
pkgs/top-level/all-packages.nix
(relVer: deriv: lib.attrsets.nameValuePair
("docbook_" + lib.strings.concatStringsSep "_" (lib.strings.splitString "." relVer))
deriv)
(callPackage ../by-name/do/docbook/versions.nix {}))
Copy link
Member

Choose a reason for hiding this comment

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

Now that I am looking for this, it looks less elegant than what I thought initially.
I am not understanding this code, and why the CI accepted this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just wanted to repeat less, but it seems to make no sense, and much complicate the code.

Copy link
Member

Choose a reason for hiding this comment

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

Because it does not conflict with the by-name RFC. But this is definitely not the intended use of by-name, imo

@rc-zb rc-zb marked this pull request as draft October 30, 2024 02:44
@rc-zb rc-zb force-pushed the refactor-and-update-docbook branch from f4175b7 to daeeebf Compare October 30, 2024 07:24
@rc-zb rc-zb marked this pull request as ready for review October 30, 2024 07:25
@rc-zb rc-zb force-pushed the refactor-and-update-docbook branch from daeeebf to 5ce5f0d Compare October 30, 2024 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndersonTorres AndersonTorres AndersonTorres left review comments

@Aleksanaa Aleksanaa Aleksanaa left review comments

@bjornfor bjornfor Awaiting requested review from bjornfor

@gebner gebner Awaiting requested review from gebner

@prusnak prusnak Awaiting requested review from prusnak

@adevress adevress Awaiting requested review from adevress

@amiloradovsky amiloradovsky Awaiting requested review from amiloradovsky

@edolstra edolstra Awaiting requested review from edolstra

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rc-zb @Aleksanaa @AndersonTorres