-
Notifications
You must be signed in to change notification settings - Fork 0
mockoon address fixed #10
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the @burgan-tech/vnext-schema dependency and synchronizes multiple Mockoon task and workflow definitions (JSON and HTTP files) to the new schema, likely adjusting mock server addresses or related endpoint configuration, with lockfile updated accordingly. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. WalkthroughThis PR replaces all Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Possibly related PRs
Poem
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (31)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Consider removing the duplicate
@burgan-tech/vnext-schemaentry from eitherdependenciesordevDependenciesinpackage.json(or clarifying why it must be in both) to reduce maintenance overhead and potential version drift.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider removing the duplicate `@burgan-tech/vnext-schema` entry from either `dependencies` or `devDependencies` in `package.json` (or clarifying why it must be in both) to reduce maintenance overhead and potential version drift.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary of ChangesHello @yilmaztayfun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the portability and deployment of the application by standardizing the address used to communicate with the Mockoon API. By replacing Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request updates the Mockoon service address from localhost to mockoon in numerous task definition and workflow test files, which is a good step for containerized environments. It also bumps the @burgan-tech/vnext-schema dependency. The changes are consistent and look correct.
My review includes a couple of suggestions to improve maintainability by avoiding hardcoded URLs. Since many of the modified files appear to be generated, these suggestions should ideally be applied to the source templates or generation scripts:
- In the JSON task definitions, I recommend using a placeholder for the Mockoon base URL instead of hardcoding it.
- In the
.httptest files, I suggest defining and using a variable for the Mockoon base URL, similar to how@baseUrlis already used.
| "type": "6", | ||
| "config": { | ||
| "url": "http://localhost:3001/api/banking/accounts/create", | ||
| "url": "http://mockoon:3001/api/banking/accounts/create", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The base URL for the mock service (http://mockoon:3001) is hardcoded. Since the PR description mentions these files might be generated, it would be best to address this in the generation script or template. Hardcoding URLs across many files can make future configuration changes difficult.
Consider using a placeholder for the base URL in the source template, for example {{mockoonBaseUrl}}, which can be substituted during the generation process. This would centralize the configuration and improve maintainability. This comment applies to all similar task definition files modified in this pull request.
|
|
||
| ### Test Get Contract Documents (Mockoon) | ||
| GET http://localhost:3001/api/contract/documents?groupCode=KG-2024-001 | ||
| GET http://mockoon:3001/api/contract/documents?groupCode=KG-2024-001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Mockoon base URL is hardcoded here. This file already uses a variable @baseUrl for the main application's URL. For better maintainability and consistency, you should define a variable for the Mockoon URL and use it for all Mockoon requests.
Since the PR description mentions these files might be generated, this change should ideally be made in the generator or template for these .http files. For example, you could add @mockoonUrl = http://mockoon:3001 at the top of the template and then use the {{mockoonUrl}} variable in your requests as suggested.
This comment applies to all .http files modified in this pull request.
GET {{mockoonUrl}}/api/contract/documents?groupCode=KG-2024-001
Summary by Sourcery
Update schema dependency version and regenerate related task and workflow configuration files.
Build:
Chores:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.