stylix icon indicating copy to clipboard operation
stylix copied to clipboard

treewide: update option deprecations

Open trueNAHO opened this issue 1 year ago • 2 comments

This patchset includes:

  • treewide: generate deprecation warnings
  • treewide: declare end-of-life for deprecated options
  • stylix: remove deprecated options reaching end-of-life

[!NOTE]

These commits should be merged individually, not squashed, to ensure a clear and easily reversible changelog. In case of change requests, review the commits separately.

trueNAHO avatar Aug 20 '24 14:08 trueNAHO

Btw @danth, what is the process for merging commits without squashing them:

These commits should be merged individually, not squashed, to ensure a clear and easily reversible changelog. In case of change requests, review the commits separately.

The "Rebase and merge" option seems to be currently disabled:

image

Otherwise, we could just manually merge them with git checkout master && git pull && git merge <BRANCH> && git push.

trueNAHO avatar Aug 22 '24 20:08 trueNAHO

I prefer squashing as the default since it keeps the history cleaner on the master branch, though I can enable merge commits if necessary. Usually anything which isn't suitable to be squashed can be made as separate PRs (personally I think this PR would be fine to squash into one commit)

danth avatar Aug 23 '24 19:08 danth

I prefer squashing as the default since it keeps the history cleaner on the master branch

Yes, squashing noisy commits without losing clarity is generally a good idea. 1 2 However, squashing https://github.com/danth/stylix/pull/519 into a single commit would result in a practically incomprehensible and revertible history.

though I can enable merge commits if necessary.

Given our low PR input rate, we may avoid merge commits with fast-forward merges (git rebase <NEWBASE> <BRANCH> && git merge --ff-only <BRANCH>) to further simplify the commit history and developer changelog.

For instance, the following merge commit from git log contains no reasoning to avoid duplicate information:

commit d042af478ce87e188139480922a3085218194106
Merge: 5ca31b6 77a6580
Author: Daniel Thwaites <[email protected]>
Date:   2024-08-23 21:17:24 +0100

    stylix: re-add `flake-utils` dependency (#515)

Instead, the details, including a breaking change, are buried under the following parent commits:

commit 9447b17f70806e987e38461c70c1de893d381a87
Author: NAHO <[email protected]>
Date:   2024-08-20 00:28:36 +0200

    stylix: re-add flake-utils dependency and interface flake systems

    Re-add the flake-utils dependency removed in commit ff5da2914cfc
    ("Remove dependency on flake-utils :heavy_minus_sign:") and interface
    flake systems using the "externally extensible flake systems" [1]
    pattern.

    [1]: https://github.com/nix-systems/nix-systems

    Closes: https://github.com/danth/stylix/issues/512
    Link: https://github.com/danth/stylix/pull/515

commit 15fed84dec2ecf19e110bad1c47292e172b46a44
Author: NAHO <[email protected]>
Date:   2024-08-19 23:42:24 +0200

    stylix: drop i686-linux architecture support

    Remove the i686-linux architecture support to match
    flake-utils.defaultSystems and primary NixOS architecture targets.

    BREAKING CHANGE: Drop support for the i686-linux architecture. Re-enable
    i686-linux support in user configurations with the "externally
    extensible flake systems" [1] pattern.

    [1]: https://github.com/nix-systems/nix-systems

    Link: https://github.com/danth/stylix/pull/515

commit ab67c509836d5292fcb645327d0432310459ab3f
Author: NAHO <[email protected]>
Date:   2024-08-20 00:05:52 +0200

    stylix: delegate to upstream default architecture list

    Delegate to the upstream default architecture list without altering the
    supported architectures.

    Link: https://github.com/danth/stylix/pull/515

commit 77a65800e6b1389f7e0a90090297048720c99017
Author: NAHO <[email protected]>
Date:   2024-08-20 14:44:09 +0200

    stylix: adopt flake-utils.lib.eachDefaultSystem

    Adopt the flake-utils.lib.eachDefaultSystem function for added features
    like '--impure' flag support [1].

    [1]: https://github.com/numtide/flake-utils/pull/115

    Link: https://github.com/danth/stylix/pull/515

Usually anything which isn't suitable to be squashed can be made as separate PRs

True.

FYI, Facebook's stacking workflow 3 can parallelize code review. For example, I initially had the following PRs on the same developer branch but split them up before submission for parallel code review:

  • https://github.com/danth/stylix/pull/506
  • https://github.com/danth/stylix/pull/513
  • https://github.com/danth/stylix/pull/514
  • https://github.com/danth/stylix/pull/515
  • https://github.com/danth/stylix/pull/517
  • https://github.com/danth/stylix/pull/518
  • https://github.com/danth/stylix/pull/519
  • https://github.com/danth/stylix/pull/520

(personally I think this PR would be fine to squash into one commit)

Agreed.

trueNAHO avatar Aug 26 '24 15:08 trueNAHO

(personally I think this PR would be fine to squash into one commit)

Agreed.

Actually, the following commits should remain seperated to distinguish between a code improvement of a previous commit (Fixes: 6858d08ed012 ("treewide: add soft deprecation dates (#506)")) and a breaking change (BREAKING CHANGE: Remove the deprecated 'stylix.palette.<BASE>' options.).

commit 4fa327f76600a1ed99c7a852d642b08f1a179c08
Author: NAHO <[email protected]>
Date:   2024-08-20 16:05:41 +0200

    stylix: remove deprecated 'stylix.palette.<BASE>' options at end-of-life

    BREAKING CHANGE: Remove the deprecated 'stylix.palette.<BASE>' options.

    Link: https://github.com/danth/stylix/pull/514

commit b85271ed7d340be9a1207aecee58893d74c90dcf
Author: NAHO <[email protected]>
Date:   2024-08-20 15:56:54 +0200

    treewide: declare end-of-life for deprecated options

    Fixes: 6858d08ed012 ("treewide: add soft deprecation dates (#506)")
    Link: https://github.com/danth/stylix/pull/514

trueNAHO avatar Aug 26 '24 15:08 trueNAHO

Changelog: v1

  • Drop commit 03222f979eac ("treewide: generate deprecation warnings")

trueNAHO avatar Aug 26 '24 15:08 trueNAHO