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

cosmic-session: init at 0-unstable-2024-01-17 #266339

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

a-kenji
Copy link
Contributor

@a-kenji a-kenji commented Nov 8, 2023

Description of changes

Needs a module to function properly.

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/)
  • 23.11 Release Notes (or backporting 23.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.

@ofborg ofborg bot requested a review from nyabinary November 8, 2023 23:17
@a-kenji a-kenji marked this pull request as ready for review November 10, 2023 20:13
@a-kenji a-kenji changed the title cosmic-session: init at unstable-2023-11-07 cosmic-session: init at unstable-2023-11-08 Nov 10, 2023
@a-kenji
Copy link
Contributor Author

a-kenji commented Nov 10, 2023

Result of nixpkgs-review pr 266339 run on x86_64-linux 1

1 package built:
  • cosmic-session

Copy link
Contributor

@nyabinary nyabinary left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@nyabinary nyabinary left a comment

Choose a reason for hiding this comment

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

Needs updating to the latest commit :)

@a-kenji
Copy link
Contributor Author

a-kenji commented Dec 3, 2023

Needs updating to the latest commit :)

Is e2d2732f819279b6f8e3f44234d59c7dc5c16721 not the latest commit?

pkgs/by-name/co/cosmic-session/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/co/cosmic-session/package.nix Show resolved Hide resolved
@a-kenji a-kenji force-pushed the init/cosmic-session branch 2 times, most recently from d3aec31 to 9049628 Compare December 5, 2023 19:04
@ofborg ofborg bot requested a review from nyabinary December 5, 2023 22:43
@nyabinary
Copy link
Contributor

pop-os/cosmic-session@b5cd1a8
Might want to look at this and see what applicable.

@a-kenji
Copy link
Contributor Author

a-kenji commented Dec 12, 2023

Thanks @nyabinary

Copy link
Contributor

@nbdd0121 nbdd0121 left a comment

Choose a reason for hiding this comment

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

This needs an extra

  postInstall = ''
    substituteInPlace $out/share/wayland-sessions/cosmic.desktop --replace '/usr/bin/start-cosmic' "$out/bin/start-cosmic"
  '';

to replace the hardcoded path in share/wayland-sessions/cosmic.desktop.

Copy link
Member

@lilyinstarlight lilyinstarlight left a comment

Choose a reason for hiding this comment

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

Looks pretty much good, thank you for your work on this!

I've left a few nits that could make it a bit clearer/follow convention better, but otherwise this looks fabulous

pkgs/by-name/co/cosmic-session/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/co/cosmic-session/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/co/cosmic-session/package.nix Outdated Show resolved Hide resolved
@nyabinary
Copy link
Contributor

Also need to update the versioning scheme to put a 0- at the front of the name and update the commit to the latest version :3

@a-kenji a-kenji force-pushed the init/cosmic-session branch 2 times, most recently from cb8e344 to 3da201d Compare January 14, 2024 14:38
@a-kenji
Copy link
Contributor Author

a-kenji commented Jan 14, 2024

Thank you for the review @lilyinstarlight !

@ofborg ofborg bot requested a review from nyabinary January 14, 2024 16:17
@a-kenji a-kenji force-pushed the init/cosmic-session branch 3 times, most recently from 002134d to 4fe2739 Compare January 16, 2024 14:08
@a-kenji a-kenji changed the title cosmic-session: init at unstable-2023-11-08 cosmic-session: init at 0-unstable-2024-01-12 Jan 16, 2024
Copy link
Member

@lilyinstarlight lilyinstarlight left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you! I will merge when ofborg is green

@nyabinary
Copy link
Contributor

nyabinary commented Jan 16, 2024

new commit dropped: pop-os/cosmic-session@334aebc

@a-kenji a-kenji force-pushed the init/cosmic-session branch 3 times, most recently from 0fcda9f to 6f61987 Compare January 16, 2024 23:58
@a-kenji a-kenji changed the title cosmic-session: init at 0-unstable-2024-01-12 cosmic-session: init at 0-unstable-2024-01-17 Jan 16, 2024
@lilyinstarlight lilyinstarlight merged commit d138050 into NixOS:master Jan 18, 2024
21 of 23 checks passed
@a-kenji a-kenji deleted the init/cosmic-session branch January 18, 2024 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants