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

chore: update nixops #24

Merged
merged 5 commits into from
Sep 27, 2024
Merged

chore: update nixops #24

merged 5 commits into from
Sep 27, 2024

Conversation

dbarrosop
Copy link
Member

@dbarrosop dbarrosop commented Sep 27, 2024

PR Type

Enhancement


Description

  • Updated Go version from 1.22 to 1.23 and related tools (golangci-lint, mockgen, gqlgen, gqlgenc, oapi-codegen) to their latest versions in overlays/go.nix.
  • Upgraded nhost-cli to version 1.24.5 and updated corresponding SHA256 hashes for all platforms in overlays/nhost-cli.nix.
  • Enhanced PostgreSQL overlay with locale support:
    • Added specific UTF-8 locales configuration.
    • Updated postFixup for all PostgreSQL versions to include locale archive wrapping.
    • Added 'dev' to outputs for better package management.
  • Modified AI review workflow configuration in .github/workflows/gen_ai_review.yaml to use 'ignore.glob' instead of 'config.ignore.glob'.

Changes walkthrough 📝

Relevant files
Dependencies
go.nix
Update Go and related tools versions                                         

overlays/go.nix

  • Updated Go version from 1.22 to 1.23
  • Updated golangci-lint, mockgen, gqlgen, gqlgenc, and oapi-codegen
    versions
  • Minor syntax improvements and variable renaming
  • +15/-15 
    nhost-cli.nix
    Upgrade nhost-cli to version 1.24.5                                           

    overlays/nhost-cli.nix

  • Updated nhost-cli version from v1.24.1 to v1.24.5
  • Updated SHA256 hashes for all platform-specific downloads
  • +10/-10 
    Enhancement
    postgres.nix
    Enhance PostgreSQL overlay with locale support                     

    overlays/postgres.nix

  • Added locales configuration for specific UTF-8 locales
  • Updated postFixup for PostgreSQL versions to include locale archive
    wrapping
  • Added 'dev' to outputs for all PostgreSQL versions
  • +35/-6   
    Configuration changes
    gen_ai_review.yaml
    Update AI review workflow configuration                                   

    .github/workflows/gen_ai_review.yaml

  • Changed 'config.ignore.glob' to 'ignore.glob' in the workflow
    configuration
  • +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Version Updates
    Multiple Go-related tools have been updated to newer versions. Ensure compatibility with the rest of the project.

    Locale Configuration
    New locale configuration added. Verify if this change affects database operations or if additional testing is required.

    PostgreSQL Modifications
    Significant changes to PostgreSQL configuration, including new postFixup steps and output modifications. Ensure these changes don't introduce unexpected behavior.

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Reduce code duplication in the postFixup hook by using a loop for wrapping multiple programs

    Consider using a loop or a helper function to reduce code duplication when wrapping
    multiple programs with the same LOCALE_ARCHIVE prefix.

    overlays/postgres.nix [96-102]

     postFixup = final.lib.optionalString final.stdenv.hostPlatform.isGnu
       ''
    -    wrapProgram $out/bin/pg_dump --prefix LOCALE_ARCHIVE ${locales}/lib/locale/locale-archive
    -    wrapProgram $out/bin/pg_dumpall --prefix LOCALE_ARCHIVE ${locales}/lib/locale/locale-archive
    -    wrapProgram $out/bin/pg_restore --prefix LOCALE_ARCHIVE ${locales}/lib/locale/locale-archive
    -    wrapProgram $out/bin/psql --prefix LOCALE_ARCHIVE ${locales}/lib/locale/locale-archive
    +    for prog in pg_dump pg_dumpall pg_restore psql; do
    +      wrapProgram $out/bin/$prog --prefix LOCALE_ARCHIVE ${locales}/lib/locale/locale-archive
    +    done
       '';
    Suggestion importance[1-10]: 7

    Why: This suggestion offers a meaningful improvement in code maintainability and readability by reducing repetition. It's a good practice that makes the code more concise and easier to update in the future.

    7
    Best practice
    Maintain consistent naming conventions for function arguments across package overrides

    Consider using a consistent naming convention for the buildGoModule argument across
    different package overrides. In the golangci-lint override, it's named
    buildGo123Module, while in others it's buildGoModule.

    overlays/go.nix [15-20]

     golangci-lint = prev.golangci-lint.override {
    -  buildGo123Module = args: final.buildGoModule (args // rec {
    +  buildGoModule = args: final.buildGoModule (args // rec {
         version = "1.61.0";
         src = prev.fetchFromGitHub {
           owner = "golangci";
           repo = "golangci-lint";
    Suggestion importance[1-10]: 6

    Why: Consistency in naming conventions is important for code readability and maintainability. This suggestion addresses a minor inconsistency that, while not critical, would improve the overall code quality and reduce potential confusion.

    6
    Enhancement
    Use a more specific condition for the postFixup hook to ensure it only applies to the intended systems

    Consider using a more specific condition for the postFixup hook. The current
    condition final.stdenv.hostPlatform.isGnu might be too broad. You could use a more
    precise condition like final.stdenv.hostPlatform.isLinux if the intention is to
    apply this only on Linux systems.

    overlays/postgres.nix [96-102]

    -postFixup = final.lib.optionalString final.stdenv.hostPlatform.isGnu
    +postFixup = final.lib.optionalString final.stdenv.hostPlatform.isLinux
       ''
         wrapProgram $out/bin/pg_dump --prefix LOCALE_ARCHIVE ${locales}/lib/locale/locale-archive
         wrapProgram $out/bin/pg_dumpall --prefix LOCALE_ARCHIVE ${locales}/lib/locale/locale-archive
         wrapProgram $out/bin/pg_restore --prefix LOCALE_ARCHIVE ${locales}/lib/locale/locale-archive
         wrapProgram $out/bin/psql --prefix LOCALE_ARCHIVE ${locales}/lib/locale/locale-archive
       '';
    Suggestion importance[1-10]: 5

    Why: The suggestion to use a more specific condition (isLinux instead of isGnu) is valid and could improve precision, but the impact is moderate as the current condition likely covers the intended systems already.

    5

    💡 Need additional feedback ? start a PR chat

    @dbarrosop dbarrosop merged commit 3355850 into main Sep 27, 2024
    2 checks passed
    @dbarrosop dbarrosop deleted the upd branch September 27, 2024 19:19
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants