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

xscreensaver: Enable perl modules needed for RSS image fetch #57629

Merged
merged 1 commit into from Mar 14, 2019

Conversation

booxter
Copy link
Contributor

@booxter booxter commented Mar 14, 2019

To validate that it works, just execute, e.g.:

$ xscreensaver-getimage-file \
    https://www.nasa.gov/rss/dyn/lg_image_of_the_day.rss

To validate that it works, just execute, e.g.:

```
$ xscreensaver-getimage-file \
    https://www.nasa.gov/rss/dyn/lg_image_of_the_day.rss
```
@booxter
Copy link
Contributor Author

booxter commented Mar 14, 2019

A backport to previous releases needs slight adjustment. Here is 18.09: https://github.com/booxter/nixpkgs/tree/release-18.09-xscreensaver-getimage-rss

@7c6f434c 7c6f434c self-assigned this Mar 14, 2019
@7c6f434c
Copy link
Member

Thanks for the fix! Does 19.03 backport require any changes? Re: 18.09 — could you please open a separate PR for these changes and mention me there?

In general, we try to prefix the commit message with the package name.

@7c6f434c 7c6f434c changed the title Enable perl modules needed for xscreensaver RSS image fetch xscreensaver: Enable perl modules needed for RSS image fetch Mar 14, 2019
@7c6f434c 7c6f434c merged commit 4730466 into NixOS:master Mar 14, 2019
@booxter
Copy link
Contributor Author

booxter commented Mar 14, 2019

@7c6f434c I honestly haven't checked 19.03 since I run 18.09 on my machine. The only potential change in backport I needed was to add explicit stdenv.lib. prefix to makePerlPath. What's the backport process in NixOS? Can I nominate a PR for backport? Can't find it described in docs. They only mention that I should cherry-pick -xe. Is it all there is? Are there criteria for acceptable backports?

@booxter booxter deleted the xscreensaver-getimage-rss branch March 14, 2019 17:42
@7c6f434c
Copy link
Member

19.03 isn't even released yet, indeed — and the process of trying is probably the same as for master but with release-19.03.

Backport process: rather informal, people see changes that seem safe and fix bugs, and port them to release branches. When a blind cherry-picking is likely to work, mentioning the idea of backporting directly is sometimes enough. In general, it is a completely normal practice to open PRs against release-… branches with changes that are either (in some form) present on master or proposed for master or make no sense for master at all but make sense for release branches. In the former cases, commit and PR references are very welcome in PR description, in the latter case, explaining why we do not want that on master but do want on release branch is of course a good idea.

@7c6f434c
Copy link
Member

And what I probably should have said — it is not like you should have found some information on backporting, it is indeed not fully documented, and attempts to formalise the workflow have not fully succeeded. If you continue to contribute to Nix* (and I hope you will), you will unfortunately see some cases where a definitive guide on something cannot exist because a full agreement on detailed enough policy has never been reached.

(And of course please do not take my question about 19.03 as a criticism that you haven't tried it — that's all OK and normal)

7c6f434c pushed a commit that referenced this pull request Mar 14, 2019
@7c6f434c
Copy link
Member

9388fc9 release-19.03

@booxter
Copy link
Contributor Author

booxter commented Mar 14, 2019

@7c6f434c thanks for the solid answer. :) I know how hard it is to formalize procedures, I've been on stable release team of OpenStack for a while.

I've noticed you merged the fix into release-19.03. Thanks! I don't see a corresponding PR posted with the backport in the list of closed PRs; am I missing something? Or is it that you have direct push access to stable branches and hence don't need a PR? (This is not to criticize or anything, just trying to check if that's what happened with the backport.)

I will follow up with a release-18.09 backport in a sec.

booxter added a commit to booxter/nixpkgs that referenced this pull request Mar 14, 2019
7c6f434c added a commit that referenced this pull request Mar 15, 2019
xscreensaver: Enable perl modules needed for RSS image fetch (#57629)
@7c6f434c
Copy link
Member

It might be a bit more annoying with Nixpkgs — we also have a vaguer set of goals so the tradeoffs are hard to even enumerate from time to time…

Committers currently have direct push access to all Nixpkgs branches (force-push is blocked on all branches anyone cares about). It is OK to merge PRs via manual cherry-picking or merging and pushing. It is OK to volunteer to do a backport via direct cherry-pick+push. It is expected that committers create PRs for large change; there is no real consensus on small fixes, so many committers push these directly.

Maybe some day improved tooling will change all this.

We also have a problem of taking too long to review some PRs (sometimes for random reasons), and the number of open PRs slowly creeps up. There are some people who do great work fighting this; last few times I tried, I burned out on that task.

For 18.09 you said you had already tested a slightly different change, so I waited to merge the different PR as it was less effort for me — thanks for the backport PR.

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.

None yet

3 participants