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

scribus: Rename scribus{,Unstable} -> scribus{_1_4,} #172700

Merged
merged 1 commit into from
May 26, 2022
Merged

scribus: Rename scribus{,Unstable} -> scribus{_1_4,} #172700

merged 1 commit into from
May 26, 2022

Conversation

turion
Copy link
Contributor

@turion turion commented May 12, 2022 •
edited
Loading

Description of changes

Since 1.5 is the current published version, and 1.4 is already quite outdated and doesn't work without friction anymore, we should rename:

  • scribus -> scribus_1_4
  • scribusUnstable -> scribus

And we should keep major version updates to scribus.

CC @MarcWeber @erictapen @Kiwi

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@turion
Copy link
Contributor Author

turion commented May 12, 2022

@ofborg build scribus scribus_1_4

@turion
Copy link
Contributor Author

turion commented May 12, 2022

What I'm unsure about is whether there should be some kind of deprecation warning for the attribute scribusUnstable.

@turion
Copy link
Contributor Author

turion commented May 12, 2022

@ofborg build scribus

@turion turion force-pushed the dev_rename_scribus branch from 9c83f0c to 5ea1fac Compare May 12, 2022 14:28
@turion
Copy link
Contributor Author

turion commented May 12, 2022

@ofborg build scribus

@ofborg ofborg bot added 8.has: clean-up 8.has: package (new) This PR adds a new package labels May 12, 2022
@ofborg ofborg bot requested review from erictapen, Kiwi and MarcWeber May 12, 2022 15:12
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels May 12, 2022
erictapen
erictapen requested changes May 12, 2022
View reviewed changes
Copy link
Member

@erictapen erictapen left a comment

Choose a reason for hiding this comment

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

This absolutely makes sense 👍

pkgs/top-level/all-packages.nix Outdated
inherit (gnome2) libart_lgpl;
};

scribusUnstable = libsForQt5.callPackage ../applications/office/scribus/unstable.nix { };
scribus = libsForQt5.callPackage ../applications/office/scribus/default.nix { };
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't we then name this scribus_1_5 directly and make scribus an alias to that? This way we won't need to shuffle everything around when 1.6 gets released.

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'm of a mind to actually delete the older scribus versions completely. But that doesn't have to happen right now.

Would you put the alias in pkgs/top-level/aliases.nix or right here?

Copy link
Member

Choose a reason for hiding this comment

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

Considering that https://www.scribus.net/downloads/ still advertises 1.4 as the stable version I'd rather keep it as long as that doesn't change. I think we just can add scribus = scribus_1_5; here in that file.
Also it might make sense to mention the change in the release notes, so users that are depending on 1.4 get warned that they need to reference it explicitly. 1.4 and 1.5 have quite different user interfaces as far as I remember.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://www.scribus.net/downloads/ still advertises 1.4 as the stable version

Thanks, I didn't notice! Will do.

Copy link
Member

Choose a reason for hiding this comment

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

scribus 1.4 depends on python2 and qt4, which have both been unsupported for years. I'm in favor of getting rid of it, even if the download page says that this is the stable version.

Copy link
Member

Choose a reason for hiding this comment

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

@ajs124 Makes sense to me, but let's discuss that in another issue.

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@turion turion force-pushed the dev_rename_scribus branch from 5ea1fac to fbbab02 Compare May 13, 2022 08:52
@turion turion requested a review from erictapen May 13, 2022 08:59
@turion turion force-pushed the dev_rename_scribus branch from fbbab02 to 1816b4e Compare May 16, 2022 09:06
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation labels May 16, 2022
@turion turion force-pushed the dev_rename_scribus branch from 1816b4e to 28c8201 Compare May 16, 2022 09:20
erictapen
erictapen approved these changes May 16, 2022
View reviewed changes
Copy link
Member

@erictapen erictapen left a comment

Choose a reason for hiding this comment

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

This looks good to me!
Also checked that this is a noop for the two scribus attributes, so nothing should break from this.
I think I'll merge in ~3 days, but I want @MarcWeber as the other maintainer to have the chance to respond to the proposed change first.

@Artturin Artturin added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels May 16, 2022
@erictapen erictapen merged commit 3b6fb49 into NixOS:master May 26, 2022
@github-actions github-actions bot mentioned this pull request May 26, 2022
Merged
1 task
@github-actions
Copy link
Contributor

Successfully created backport PR #174786 for release-22.05.

@turion turion deleted the dev_rename_scribus branch May 30, 2022 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erictapen erictapen erictapen approved these changes

@ajs124 ajs124 ajs124 left review comments

@Kiwi Kiwi Awaiting requested review from Kiwi

@MarcWeber MarcWeber Awaiting requested review from MarcWeber

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: clean-up 8.has: documentation This PR adds or changes documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@turion @ajs124 @erictapen @Artturin