Skip to content
This repository was archived by the owner on Jun 11, 2025. It is now read-only.

script url will be provided along with curl#366

Merged
nxtcoder36 merged 2 commits into
release-v1.0.7from
ref/registry-image-url
Sep 11, 2024
Merged

script url will be provided along with curl#366
nxtcoder36 merged 2 commits into
release-v1.0.7from
ref/registry-image-url

Conversation

@nxtcoder36
Copy link
Copy Markdown
Contributor

@nxtcoder36 nxtcoder36 commented Sep 11, 2024

Summary by Sourcery

Refactor the CoreGetRegistryImageURL function to remove unnecessary parameters and update the GraphQL schema accordingly. Introduce a new environment variable for script URLs and adjust related code to accommodate these changes.

Enhancements:

  • Simplify the CoreGetRegistryImageURL function by removing the image and meta parameters, and update related function calls and GraphQL schema.
  • Add a new environment variable WebhookScriptURL to the Env struct for handling script URLs.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Sep 11, 2024

Reviewer's Guide by Sourcery

This pull request modifies the CoreGetRegistryImageURL function to remove the image and meta parameters, simplifying the API. It also introduces a new scriptUrl field in the RegistryImageURL struct and updates the implementation to provide both a curl command and a script URL for image retrieval.

File-Level Changes

Change Details Files
Simplified CoreGetRegistryImageURL function signature
  • Removed 'image' and 'meta' parameters from CoreGetRegistryImageURL function
  • Updated function calls and implementations to reflect the new signature
  • Modified GraphQL schema to remove 'image' and 'meta' arguments from core_getRegistryImageURL query
apps/console/internal/app/graph/generated/generated.go
apps/console/internal/domain/registry-image.go
apps/console/internal/app/graph/schema.resolvers.go
apps/console/internal/domain/api.go
Added scriptUrl field to RegistryImageURL struct
  • Introduced scriptUrl field in RegistryImageURL struct
  • Updated GetRegistryImageURL function to populate both URL and scriptUrl fields
  • Added constant for RegistryImageURLScriptUrl in field constants
apps/console/internal/domain/registry-image.go
apps/console/internal/entities/field-constants/generated_constants.go
Updated environment variables
  • Added WebhookScriptURL to Env struct
apps/console/internal/env/env.go
Modified curl command generation
  • Updated URL generation to use a fixed format instead of dynamic image and meta values
  • Added scriptUrl generation using the new WebhookScriptURL environment variable
apps/console/internal/domain/registry-image.go
Updated GraphQL query in test file
  • Removed 'image' and 'meta' variables from Core_getRegistryImageURL query
  • Updated global image variable
.tools/nvim/__http__/console/registry-image.graphql.yml

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey @nxtcoder36 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Could you provide more context on why this change was made? It seems to be moving from a dynamic approach to a more static one, which might affect flexibility.
  • Consider parameterizing the hardcoded values in the GetRegistryImageURL function instead of using static strings for image name, tag, and meta information.
  • Please ensure that the new WebhookScriptURL environment variable is properly documented and set in all necessary environments.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@nxtcoder36 nxtcoder36 merged commit 7e90c2f into release-v1.0.7 Sep 11, 2024
@nxtcoder36 nxtcoder36 deleted the ref/registry-image-url branch September 11, 2024 09:29
abdheshnayak pushed a commit that referenced this pull request Nov 5, 2024
* script url will be provided along with curl

* script url is hosted in webhook
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants