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

sqruff: init at 0.17.0 #342435

Merged
merged 3 commits into from
Oct 17, 2024
Merged

sqruff: init at 0.17.0 #342435

merged 3 commits into from
Oct 17, 2024

Conversation

Hasnep
Copy link
Contributor

@Hasnep Hasnep commented Sep 17, 2024

Description of changes

sqruff is a fast SQL linter and formatter.

fix #336098

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Sep 17, 2024
@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Sep 17, 2024
@Hasnep Hasnep marked this pull request as ready for review September 17, 2024 04:02
Copy link
Member

@Bot-wxt1221 Bot-wxt1221 left a comment

Choose a reason for hiding this comment

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

Correct the PR title and commit message. Otherwise, LGTM. Link the PR with #336098 by adding fix https://github.com/NixOS/nixpkgs/issues/336098 to your description of PR.

@Hasnep Hasnep changed the title Init sqruff at v0.15.8 sqruff: init at v0.17.0 Sep 19, 2024
@Hasnep
Copy link
Contributor Author

Hasnep commented Sep 19, 2024

Thanks @Bot-wxt1221, I've edited the commit messages, PR title and added a link to the relevant issue.

Copy link
Member

@Bot-wxt1221 Bot-wxt1221 left a comment

Choose a reason for hiding this comment

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

LGTM

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 19, 2024
pkgs/by-name/sq/sqruff/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/sq/sqruff/package.nix Outdated Show resolved Hide resolved
@GaetanLepage
Copy link
Contributor

Commit should be name: sqruff: init at 0.17.0 (without the 'v').

@Bot-wxt1221
Copy link
Member

Build fail. Read ofborg for detailed information. https://github.com/NixOS/nixpkgs/pull/342435/checks?check_run_id=30386821511

@Hasnep Hasnep changed the title sqruff: init at v0.17.0 sqruff: init at 0.17.0 Sep 27, 2024
@GaetanLepage
Copy link
Contributor

Fails with:

   Compiling sqruff-lib v0.17.0 (/build/source/crates/lib)
error[E0412]: cannot find type `PanicHookInfo` in module `panic`
   --> crates/lib/src/helpers.rs:141:46
    |
141 |         let hook = move |panic_info: &panic::PanicHookInfo<'_>| {
    |                                              ^^^^^^^^^^^^^ help: a struct with a similar name exists: `PanicInfo`
   --> /build/rustc-1.80.1-src/library/core/src/panic/panic_info.rs:26:1
    |
    = note: similarly named struct `PanicInfo` defined here

For more information about this error, try `rustc --explain E0412`.
error: could not compile `sqruff-lib` (lib) due to 1 previous error

@Hasnep
Copy link
Contributor Author

Hasnep commented Sep 27, 2024

Very strange, it was working on my laptop earlier, maybe rebasing broke it? I'll see if I can push a fix :/

@Hasnep
Copy link
Contributor Author

Hasnep commented Sep 27, 2024

Ah, it's because originally I based this branch on the staging branch which has Rust 1.81 (#339854). The master branch still has Rust 1.80, so this PR can't be merged until that's updated.

@GaetanLepage
Copy link
Contributor

Ah, it's because originally I based this branch on the staging branch which has Rust 1.81 (#339854). The master branch still has Rust 1.80, so this PR can't be merged until that's updated.

Oh sure ! staging-next is only a few days away from being merged into master so this shouldn't be too long.

Feel free to ping me again when the time has come.

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 29, 2024
@Hasnep
Copy link
Contributor Author

Hasnep commented Oct 17, 2024

@GaetanLepage I believe this PR is ready for review again :)

@Hasnep Hasnep force-pushed the add-sqruff branch 2 times, most recently from 98456ff to a959c18 Compare October 17, 2024 14:53
Co-authored-by: Gaétan Lepage <33058747+GaetanLepage@users.noreply.github.com>
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

LGTM !

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 342435


x86_64-linux

✅ 1 package built:
  • sqruff

aarch64-linux

✅ 1 package built:
  • sqruff

x86_64-darwin

✅ 1 package built:
  • sqruff

aarch64-darwin

✅ 1 package built:
  • sqruff

@GaetanLepage GaetanLepage merged commit 1d8bdbd into NixOS:master Oct 17, 2024
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package request: sqruff
5 participants