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

fix: gateway supergraph build script updates#296

Merged
nxtcoder17 merged 1 commit into
mainfrom
fix/gateway-supergraph
Mar 12, 2024
Merged

fix: gateway supergraph build script updates#296
nxtcoder17 merged 1 commit into
mainfrom
fix/gateway-supergraph

Conversation

@nxtcoder17
Copy link
Copy Markdown
Member

No description provided.

@nxtcoder17 nxtcoder17 requested a review from karthik1729 as a code owner March 12, 2024 11:37
@nxtcoder17 nxtcoder17 merged commit f4368b2 into main Mar 12, 2024
@nxtcoder17 nxtcoder17 deleted the fix/gateway-supergraph branch March 12, 2024 11:38
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 @nxtcoder17 - I've reviewed your changes and they look great!

General suggestions:

  • Consider validating the 'out' variable in the generate-schema.sh script to ensure it handles paths with spaces or special characters gracefully.
  • Review the changes to ensure that the dynamic output path for the schema generation does not introduce any unexpected behavior in different environments.
  • Ensure that the removal of the Generate Schema step in the GitHub workflow does not impact other dependent processes or workflows.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Docstrings: all looks good

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

cat ../message-office/internal/app/graph/*.graphqls >./schemas/message-office-api.schema

rover supergraph compose --config ./supergraph.yml --output prod-schema.graphql --elv2-license accept
rover supergraph compose --config ./supergraph.yml --output "$out" --elv2-license accept
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (code_refinement): It's good practice to quote shell variables to prevent globbing and word splitting. However, ensure that the variable out is always expected to be a single word or path without spaces, or this could lead to unexpected behavior.

Suggested change
rover supergraph compose --config ./supergraph.yml --output "$out" --elv2-license accept
if [[ "$out" =~ \ |\' ]]; then
echo "Error: The output path ('out') contains spaces or single quotes, which might cause unexpected behavior."
exit 1
fi
rover supergraph compose --config ./supergraph.yml --output "$out" --elv2-license accept

abdheshnayak pushed a commit that referenced this pull request Nov 5, 2024
fix: gateway supergraph build script updates
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.

1 participant