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.

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

morgen: 3.5.9 -> 3.6.6 #367347

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

morgen: 3.5.9 -> 3.6.6 #367347

wants to merge 1 commit into from

Conversation

Jegp
Copy link

@Jegp Jegp commented Dec 22, 2024

Upgrades Morgen from 3.5.9 to 3.6.6

This is a follow-up of #361802

I tried making the PR as skimmable and easy-to-process as possible. I would love to hear how I can make the process easier on the maintainers. Thank you for the work you're doing here!

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@felbinger
Copy link
Member

I tried making the PR as skimmable and easy-to-process as possible.

You failed :)
image

You made the nix expression invalid.

Check the CI pipeline, after proposing the change, if it fails fix it!

If your confident, that your changes are ready to be merged contact the maintainers of the package (here: justanotherariel, see meta section of the file you edited...)

And again: Do not create a new pr for each version bump because a new patch has been released, simply update your pr.

@Jegp
Copy link
Author

Jegp commented Dec 22, 2024

You failed :) image

You made the nix expression invalid.

Thank you! I fixed and tested the changes locally. I'll keep an eye on the checks.

If your confident, that your changes are ready to be merged contact the maintainers of the package (here: justanotherariel, see meta section of the file you edited...)

Thanks, that's helpful because it gives me agency. If all the tests pass, I'll reach out to them.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 22, 2024
};

This comment was marked as outdated.

@felbinger
Copy link
Member

felbinger commented Dec 22, 2024

Looks good so far, ofborg will take very long time to finish, but your change looks good. If you tested it locally and it all still works (which is the case usually for simple patch updates) you can ping the maintainer to check it too.
Due to the fact that morgen hasn't been moved to the new structure (pkgs/by-name) yet, he (at least as a simple package maintainer) won't be able to merge it though (He may have merge rights for the whole repo, I didn't check). With the new structure package maintainers can merge their packages themself using a bot.

@Jegp
Copy link
Author

Jegp commented Dec 23, 2024

Line re-introduced :-)

Looks good so far, ofborg will take very long time to finish, but your change looks good. If you tested it locally and it all still works (which is the case usually for simple patch updates) you can ping the maintainer to check it too. Due to the fact that morgen hasn't been moved to the new structure (pkgs/by-name) yet, he (at least as a simple package maintainer) won't be able to merge it though (He may have merge rights for the whole repo, I didn't check). With the new structure package maintainers can merge their packages themself using a bot.

Thanks for the information. I'm learning a lot! But if that's the case, shouldn't I just move the whole thing to pkgs/by-name/mo/morgen?

@justanotherariel
Copy link
Contributor

justanotherariel commented Dec 23, 2024

Yea, I planned doing that at some point but didn't get to it. Sorry for the late reply - things have been very busy lately. Seems like something broke in the auto update script... that needs fixing too, then ryantm will take care of creating these PRs.
Please fix the failing pipeline (trailing whitespace), after which I can take a look. I'll then make my own PR moving morgen to the pkgs dir and fixing the update script - hopefully in the next couple of days.
Cheers

@justanotherariel
Copy link
Contributor

And you can (and should) test your chages in nixpkgs by checking out the repository/branch, and then executing nix run .#morgen in that directory, if you have nix experimental commands enabled. There's probably a way to do it with nix-shell too. If you need an update quickly, you can include your own git patches when importing nixpkgs in your flake - but that may be a bit too advanced.

@Jegp
Copy link
Author

Jegp commented Jan 6, 2025

No worries about a late reply. This is volunteer work. I must admit the process here is confusing to me, but I am just trying to be helpful.

I did test it locally. That's the version I'm running now.

I believe the trailing whitespace was fixed after I addressed the comment from @felbinger

@Jegp
Copy link
Author

Jegp commented Feb 14, 2025

Branch has been updated with latest version of Morgen, 3.6.6

@Jegp Jegp changed the title morgen: 3.5.9 -> 3.6.3 morgen: 3.5.9 -> 3.6.6 Feb 14, 2025
@felbinger
Copy link
Member

felbinger commented Feb 14, 2025

Branch has been updated with latest version of Morgen, 3.6.6

Please rebase, merge commits aren't allowed!
image

# first drop the merge commit
git reset --hard HEAD~1

# then rebase your branch with official nixpkgs
git remote add upstream [email protected]:nixos/nixpkgs
git fetch --all
git rebase upstream/master

# and force push
git push -f

@felbinger
Copy link
Member

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 367347

Logs: https://github.com/felbinger/nixpkgs-review-gha/actions/runs/13330340733


x86_64-linux

✅ 1 package built:
  • morgen

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 21, 2025
@github-actions github-actions bot added 6.topic: dotnet Language: .NET 6.topic: nvidia 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: teams Relating to team creation, updates, other management actions and removed 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: emacs Text editor 6.topic: printing 6.topic: golang 6.topic: vim 6.topic: hardware 6.topic: testing Tooling for automated testing of packages and modules 6.topic: module system About "NixOS" module system internals 6.topic: jitsi 6.topic: nim Nim programing language 6.topic: flakes The experimental Nix feature 6.topic: lib The Nixpkgs function library 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 6.topic: k3s Kubernates distribution (https://k3s.io/) 6.topic: deepin Desktop environment and its components 6.topic: dotnet Language: .NET 6.topic: nvidia 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: teams Relating to team creation, updates, other management actions labels Feb 21, 2025
@Jegp
Copy link
Author

Jegp commented Feb 21, 2025

Branch has been updated with latest version of Morgen, 3.6.6

Please rebase, merge commits aren't allowed!

Turns out that rebasing was a pretty poor strategy. It added a ton of partial or unresolved commits. Which surprises me, because I would assume nixpkgs commit history shouldn't contain that kind of discrepancies. Do you know why that's the case?

Anyway, I reset to nixpkgs master and cherry-picked the commit for Morgen. So it should be up-to-date now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants