-
-
Notifications
You must be signed in to change notification settings - Fork 797
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
Usertype removal - part 1 #1858
Usertype removal - part 1 #1858
Conversation
Added support to add members in an organization (PalisadoesFoundation#1719)
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. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
@pranshugupta54 Please fix the failing tests |
|
The lint check is too ensure we have consistent coffee quality. It is therefore relevant to the PR |
@palisadoes, we should take it separately. I haven't touched the functions which had linting issues, but fixed userType in those files. These eslint issues are there in the codebase for so long and should be taken in different issue or should be fixed by users working on the file. It's also going to take one person very long to understand each function of each file to fix those types. |
Hey can you please tell me why is the introspection failing? Because it was fixed, and now it is the duty of all new future PR's to ensure that it passes, otherwise it would nullify the entire point of having it in the workflow. Thank You. |
Yes I agree with you on this point. Because the codebase has a lot of existing lint issues, and all of them cannot be solved trivially as one has to understand the context of each line to replace the 'any' types. I don't think having the eslint disable workflow as a blocker for the testing is the right choice. It should be there to notify the reviewers about its usage, but should not block the tests from running. Thank you. |
|
The application tests won't be completed until the linting is done. This has been in place in this repo for months |
@palisadoes, I've reverted 4 files to fix eslint issue. We can take those files later. Everything is passing well, except irrelevant introspection test. |
1d057d9
into
PalisadoesFoundation:develop
Ref #1827
appUserProfileId
field which returnsnull
always.// eslint-disable-line
to some lines which should be fixed separately (userType changes require me to fix lint in that file which is not relevant to this issue)Files:
updateUserType
mutation talawa-api#2139updateUserType
mutation talawa-api#2139