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

azure-cli: move to by-name, nixfmt #325950

Merged
merged 3 commits into from
Jul 10, 2024

Conversation

superherointj
Copy link
Contributor

azure-cli: move to by-name, nixfmt

  • move from top-level to by-name
  • run nixfmt on nix files (but not on automated file): extensions-manual.nix, package.nix, python-packages.nix
    I have not included extensions-generated.nix at this point in time.

Goal here is minimalist, to just establish a minimal base for a subsequent refactor.

CC @katexochen @obreitwi

@katexochen

This comment was marked as resolved.

@emilazy
Copy link
Member

emilazy commented Jul 10, 2024

FWIW: The treewide format PR won’t touch files that have had open PRs in the past while, and will enforce formatting changed files on every new PR. So it wouldn’t reformat this file now, and it would actually enforce doing what @superherointj did here. The message you quoted was in the context of whether to hold off on doing mass reformats before the formatter exactly matches the RFC formatting guidelines, as an argument for reformatting as much of the tree ASAP as possible; see e.g. #322537 (comment). There’s already been a lot of small‐scale formatting and I’m not sure it makes sense to add every single commit to .git-blame-ignore-revs. That said, it might be best to any include any further refactoring in this PR rather than touching the files just for mechanical changes?

@katexochen
Copy link
Contributor

I’m not sure it makes sense to add every single commit to .git-blame-ignore-revs

@emilazy I really think we should add it, git blame gives a lot of helpful info for packages of this size.

@emilazy
Copy link
Member

emilazy commented Jul 10, 2024

Ah, I missed just how big this is :) It may be a good idea, then. Though I wonder how well git blame works even then with the combination of a move and big reformat; .git-blame-ignore-revs isn’t perfect in my experience.

@superherointj
Copy link
Contributor Author

The repo will be formatted anyway. Which can cause breakages, for example, update scripts (#310700).
We are better knowing how things are actually going to be.

I will add the commit to .git-blame-ignore-revs then.

@katexochen katexochen merged commit 91e135a into NixOS:master Jul 10, 2024
23 of 25 checks passed
@superherointj superherointj deleted the azure-cli-refactor branch July 10, 2024 11:49
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.

4 participants