-
-
Notifications
You must be signed in to change notification settings - Fork 712
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
fixes graphql-introspection #1433
fixes graphql-introspection #1433
Conversation
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersWhen your PR has been assigned reviewers contact them to get your code reviewed and approved via:
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
Hey @palisadoes, Here are one of the of the example:
Thank You : ) |
It's failing. You may need to be using $FULL_BRANCH_NAME instead. The echo of $BRANCH_NAME isn't working
You'll need to do further commits. If it's working the introspection test must pass as part of this PR |
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1433 +/- ##
========================================
Coverage 97.34% 97.34%
========================================
Files 136 136
Lines 3572 3572
Branches 1039 1039
========================================
Hits 3477 3477
Misses 90 90
Partials 5 5 ☔ View full report in Codecov by Sentry. |
|
Also, merge this file into I see the hard coded I'll need an answer before I merge this Are you sure this is the source of the current problem? |
|
Does this make sense to you for the long term health of the project? We don't want a solution that we'll need to fix repeatedly. Are these suggestions clear? |
Yeah, got it. It helps us to manage our push workflow more easily. 🚀 |
Should I create a new issue for the inspect.yml file merging? |
Yes, a single issue to integrate inspect.yml into push.yml and to fix the automatic branch detection that will be required |
Why is this the introspection test failing in this PR if it is supposed to be pulling from the |
As I mentioned earlier in the above comment, some queries and mutation files are missing in the API develop branch. When the workflow runs for auto-generation, it only updates or generates files that are present. However, those schema, mutations, and queries exist in talawa-admin. And because of this test are failing. Although the test failure related to |
OK, thanks for reiterating the rationale. Please open the other issue and ask to be assigned to it |
5bae8f5
into
PalisadoesFoundation:develop
@palisadoes, I've reviewed the commit history and discovered that there's no need to create a new issue for this. The mutations and queries were deleted as part of deprecating the task and project features. In talawa-api the issue is resolved already. For Talawa-Admin, the problem still persists (#1362). Once this issue is resolved, our graphql-introspection tests will pass successfully 🎉 Talawa-API
Talawa-Admin
|
It's not just the mutations. We still need to make sure the GitHub action in the API automatically adjusts for the branch and consolidate the YAML into a single file. Please open an issue to do this. |
|
Understood. Please proceed. |
What kind of change does this PR introduce?
Issue Number:
Fixes #1429
Did you add tests for your changes?
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?