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

ibus-engines.table: 1.9.20 -> 1.9.21 #53080

Merged
merged 2 commits into from Jan 29, 2019
Merged

Conversation

laMudri
Copy link
Contributor

@laMudri laMudri commented Dec 30, 2018

Motivation for this change

Regular update, except for the change to config format mentioned in the release notes.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@joachifm
Copy link
Contributor

Needs rebasing

@laMudri
Copy link
Contributor Author

laMudri commented Jan 25, 2019

Is there a practical way to do this with changes to the changelog?

@joachifm
Copy link
Contributor

IMO it makes sense to make these changes separately

@laMudri
Copy link
Contributor Author

laMudri commented Jan 27, 2019

Which changes, and how could they be separate? The update is a fairly major one, which requires the note in the changelog. It doesn't make sense to separate them.

@joachifm
Copy link
Contributor

I took your question to mean that you wanted a way to avoid conflicts. If you insist on keeping the changes in the same commit, there appears to be no way around resolving the conflict, i.e. rebase ...

@laMudri
Copy link
Contributor Author

laMudri commented Jan 27, 2019

Splitting this into two commits isn't going to change the fact that it's vulnerable to conflicts in the release notes. I'd be happy to rebase, but won't that create a new commit each time I do? If the extra commits are fine, I'll just do it.

@joachifm
Copy link
Contributor

joachifm commented Jan 29, 2019

The rebase creates a new changeset, so does not entail superflous commits (perhaps you're thinking of merging master into the PR branch?). You will need to force-push the branch after rebasing, however.

I generally disagree that you necessarily need to check in both the bump and the changelog to master at the same time. The changelog pertains to the next NixOS release; to my mind, preparing that changelog is a separate concern from ordinary package bumps.
People running NixOS from random checkouts must be expected to read the git log (which I think is the appropriate medium for incompatibility notices). In that sense, splitting things up does help, because it doesn't block updating a package due to conflicts in (what I consider) an unrelated piece of the repo (the NixOS changelog).

If you want to solve the underlying problem of incompatible edits, maybe propose per-package changelogs/warnings or something. Some mechanism for delivering important info pertaining to package updates to end-users apart from the git log would be great (ala eselect news), but that seems outside the scope of this PR.

@laMudri
Copy link
Contributor Author

laMudri commented Jan 29, 2019

Aah, okay, I understand what you're intending now. I'll make those changes to this PR shortly...

@laMudri
Copy link
Contributor Author

laMudri commented Mar 10, 2019

Sorry, I didn't test this thoroughly enough. I suggest reverting, unless a quick fix for #56621 can be done.

@joachifm
Copy link
Contributor

@laMudri if a short-term fix turns out to be impossible, please submit a revert PR & I'll be happy to merge it. I suppose it needs to be reverted from the upcoming release branch as well (I've not checked if it's in there)

@laMudri laMudri mentioned this pull request Mar 15, 2019
10 tasks
@jtojnar
Copy link
Contributor

jtojnar commented Mar 15, 2019

Is it just executables (no loadable modules)? Then adding wrapGAppsHook should work.

@laMudri
Copy link
Contributor Author

laMudri commented Mar 19, 2019

@jtojnar It is just executables, but I tried adding wrapGAppsHook to nativeBuildInputs but it didn't solve the issue. Possibly gconf is involved too, though it's difficult to find the time to work it out right now.

@jtojnar
Copy link
Contributor

jtojnar commented Mar 19, 2019

Is the error still the same? I can see that adding wrapGAppsHook to nativeBuildInputs includes the GSettings schema path in the wrapper and that there are gschemas.compiled so everything looks fine.

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

4 participants