Skip to content

Comments

gobble: init at 1.3#251335

Merged
K900 merged 1 commit intoNixOS:masterfrom
VuiMuich:init/gobble
Aug 29, 2023
Merged

gobble: init at 1.3#251335
K900 merged 1 commit intoNixOS:masterfrom
VuiMuich:init/gobble

Conversation

@VuiMuich
Copy link
Contributor

@VuiMuich VuiMuich commented Aug 25, 2023

https://github.com/EmperorPenguin18/gobble

Description of changes

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/)
  • 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.

@VuiMuich
Copy link
Contributor Author

VuiMuich commented Aug 25, 2023

Depends on #250939 for maintainers-list.nix

@VuiMuich VuiMuich mentioned this pull request Aug 25, 2023
Merged
cafkafk
cafkafk requested changes Aug 25, 2023
View reviewed changes
Copy link
Member

@cafkafk cafkafk left a comment

Choose a reason for hiding this comment

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

#250939 MUST be merged before this, code looks good thou 👍

@VuiMuich
Copy link
Contributor Author

VuiMuich commented Aug 25, 2023

#250939 MUST be merged before this, code looks good thou 👍

Thanks for the review!

This was referenced Aug 25, 2023
Merged
Closed
@AndersonTorres AndersonTorres marked this pull request as draft August 25, 2023 11:35
@AndersonTorres AndersonTorres added the 2.status: blocked by pr/issue Another PR or issue is preventing this from being completed label Aug 25, 2023
@VuiMuich VuiMuich force-pushed the init/gobble branch 2 times, most recently from c9ec48b to b022f2d Compare August 25, 2023 12:32
@ofborg ofborg bot added the ofborg-internal-error Ofborg encountered an error label Aug 25, 2023
@VuiMuich VuiMuich marked this pull request as ready for review August 28, 2023 11:15
@VuiMuich
Copy link
Contributor Author

VuiMuich commented Aug 28, 2023

As the the dependent PR is accepted and can be expected to get merged soon, I feel this one is ready for review.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Aug 28, 2023
@VuiMuich VuiMuich removed the 2.status: blocked by pr/issue Another PR or issue is preventing this from being completed label Aug 28, 2023
@VuiMuich
Copy link
Contributor Author

VuiMuich commented Aug 29, 2023

@cafkafk may I ask for your assistence, please?
How ist this PR supposed to proceed?
I hope id was ok, that I removed the block by PR label myself after the blocking PR got merged and I was invited as org member.

@cafkafk
Copy link
Member

cafkafk commented Aug 29, 2023 •
edited
Loading

@cafkafk may I ask for your assistence, please? How ist this PR supposed to proceed?

Sorry, I've been kinda sick, meant to look at this later but forgot.

I hope id was ok, that I removed the block by PR label myself after the blocking PR got merged and I was invited as org member.

yea that's probably not an issue.

cafkafk
cafkafk approved these changes Aug 29, 2023
View reviewed changes
Copy link
Member

@cafkafk cafkafk left a comment

Choose a reason for hiding this comment

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

LGTM 👍 (assuming ofborg checks pass)

@VuiMuich
Copy link
Contributor Author

VuiMuich commented Aug 29, 2023

Sorry, I've been kinda sick, meant to look at this later but forgot.

I am sorry to hear, I hope you are getting better!

yea that's probably not an issue.

Oh, thanks. I was not sure how it's supposed to be handled with those lables, would usually ofborg remove the blocking lable? Also what about the internal ofborg error? Should it be removed as well, or is this ment for ofborg devs to track down issues?
Speaking of that last check, its been in those state for more then 24hs which is not really supposed to take that long isn't it? Also I am not sure, why it is trying to run tests on aarch64-darwin as this package should be (at least for now) be linux only, as I have no idea if gobble would work at all on darwin in general.

@cafkafk
Copy link
Member

cafkafk commented Aug 29, 2023

Should it be removed as well, or is this ment for ofborg devs to track down issues?

Should be fine I think.

Speaking of that last check, its been in those state for more then 24hs which is not really supposed to take that long isn't it? Also I am not sure, why it is trying to run tests on aarch64-darwin as this package should be (at least for now) be linux only, as I have no idea if gobble would work at all on darwin in general.

I'll ask in the matrix room to get someone with the right perms to come and have it re-evaluate.

@K900
Copy link
Contributor

K900 commented Aug 29, 2023

ofborg always evaluates all four platforms and skips ones the package isn't supported on at build time, as you can see by "skipped - no attempt" on the x86_64-darwin. aarch64-darwin capacity is just severely limited, but it's going to get skipped anyway.

@VuiMuich
Copy link
Contributor Author

VuiMuich commented Aug 29, 2023

I see, thanks, so basically wait out the aarch64-darwin skip and should be all greens, right?
Thanks for your fast reply!

@K900
Copy link
Contributor

K900 commented Aug 29, 2023

There's no real reason to wait it out.

@K900 K900 merged commit ead8e2d into NixOS:master Aug 29, 2023
@VuiMuich
Copy link
Contributor Author

VuiMuich commented Aug 29, 2023

Then thanks for the merge 😀

@VuiMuich VuiMuich deleted the init/gobble branch August 29, 2023 15:15
@cole-h cole-h removed the ofborg-internal-error Ofborg encountered an error label Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cafkafk cafkafk cafkafk approved these changes

Assignees

No one assigned

Labels

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 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@VuiMuich @cafkafk @K900 @AndersonTorres @cole-h