Skip to content
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

Improve contribution guide #7075

Merged
merged 8 commits into from
Feb 9, 2021

Conversation

mtrezza
Copy link
Member

@mtrezza mtrezza commented Dec 15, 2020

New Pull Request Checklist

Issue Description

The contribution guide is missing documented processes for:

  • Add new Parse Error
  • Add new Parse Server configuration parameter

This sometimes resulted in incomplete implementations in the past.

Related issue: #7100

Approach

Added processes.

TODOs before merging

(none)


Edit: This originally larger PR has been broken up into smaller tasks by moving the discussion into issue #7100.

* commit 'ccb045b68c5b4d983a90fa125513fc476e4e2387':
  fix: upgrade @graphql-tools/links from 6.2.4 to 6.2.5 (parse-community#7007)
  fix: upgrade pg-promise from 10.7.0 to 10.7.1 (parse-community#7009)
  fix: upgrade jwks-rsa from 1.10.1 to 1.11.0 (parse-community#7008)
  fix: upgrade graphql from 15.3.0 to 15.4.0 (parse-community#7011)
  update stale bot (parse-community#6998)
  fix(beforeSave/afterSave): Return value instead of Parse.Op for nested fields (parse-community#7005)
  fix(beforeSave): Skip Sanitizing Database results (parse-community#7003)
  Fix includeAll for querying a Pointer and Pointer array (parse-community#7002)
  Init (parse-community#6999)
@codecov
Copy link

codecov bot commented Dec 15, 2020

Codecov Report

Merging #7075 (a1931a1) into master (f01059f) will increase coverage by 8.44%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7075      +/-   ##
==========================================
+ Coverage   85.41%   93.85%   +8.44%     
==========================================
  Files         169      169              
  Lines       12425    12425              
==========================================
+ Hits        10613    11662    +1049     
+ Misses       1812      763    -1049     
Impacted Files Coverage Δ
src/Controllers/SchemaController.js 97.32% <0.00%> (+0.38%) ⬆️
src/RestWrite.js 93.84% <0.00%> (+0.64%) ⬆️
src/GraphQL/loaders/defaultGraphQLTypes.js 97.06% <0.00%> (+0.73%) ⬆️
src/Controllers/DatabaseController.js 95.31% <0.00%> (+1.24%) ⬆️
src/triggers.js 94.60% <0.00%> (+1.40%) ⬆️
src/ParseServerRESTController.js 98.36% <0.00%> (+1.63%) ⬆️
src/batch.js 92.30% <0.00%> (+1.92%) ⬆️
src/Controllers/FilesController.js 94.00% <0.00%> (+2.00%) ⬆️
src/Config.js 90.47% <0.00%> (+2.04%) ⬆️
src/Routers/UsersRouter.js 94.37% <0.00%> (+3.12%) ⬆️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f01059f...a1931a1. Read the comment docs.

@mtrezza mtrezza marked this pull request as draft December 15, 2020 15:14
@dblythy
Copy link
Member

dblythy commented Dec 16, 2020

@mtrezza would you see any value in including some basic fork / commit / submit a PR instructions using a visual tool such as GitHub desktop? I know for me understanding git was a challenge at the start.

@mtrezza
Copy link
Member Author

mtrezza commented Dec 16, 2020

That's a good idea, I actually think screenshots and code examples are key here. It may be the flood of text that discourages some new developers from getting into contributing. It surely must be for people who tend to absorb information visually. If we have some extra time, I would like to add some tutorial videos as well.

Would you want to write the 1-2-3 instructions for some parts? Thinking of a storyboard, we could have roughly these scenes:

  1. Clone repo locally using GitHub Desktop
  • install GitHub Desktop
  • clone repo
  • create new branch
  1. Set up Visual Studio Code to debug Parse Server
  2. Debugging Parse Server
  • create failing test cases
  • inspecting code to fix
  1. Submit PR
  • commit changes
  • create PR
  • how to restart CI in case of flaky tests
  • code coverage

I'm thinking the guide should be focused on bug fixing. We've had many instances where obvious small bugs were identified that would have been easy to fix, but the bug reporter did not know how to do that. The result was that the bugs stayed unfixed for longer than necessary.

Additional guides regarding:

  • code style
  • test cases
  • legal frameworks for contributions

Additional processes for:

  • add new Parse Error
  • add new Parse Server Configuration
  • ...

@dblythy
Copy link
Member

dblythy commented Dec 16, 2020

I'm thinking the guide should be focused on bug fixing. We've had many instances where obvious small bugs were identified that would have been easy to fix, but the bug reporter did not know how to do that. The result was that the bugs stayed unfixed for longer than necessary.

100% agree. The beauty of this package is that it is accessible to developers of all skill levels, which often means that some terminology or processes might be hard to understand for some new developers.

I would be more than happy to write instructions if you'd require me to. I was thinking I could tackle #7058 (as I think it will be a pretty easy fix) and walk through the entire process from fork, spec, PR, merge, etc, and maybe that could be as a blog post on blog.parseplatform.org?

@mtrezza
Copy link
Member Author

mtrezza commented Dec 16, 2020

I was thinking I could tackle #7058 (as I think it will be a pretty easy fix) and walk through the entire process from fork, spec, PR, merge, etc

Do you mean as a screen recording or why do you mention the specific issue #7058?
I am thinking now, how step-by-step instructions, screenshots and/or tutorial videos would best play together here. Still not sure if we should for example skip screenshots, keep the written instructions to a minimum and use the video format more instead? But we would need to find someone who would be willing to do the screencasts.

maybe that could be as a blog post on blog.parseplatform.org?

Good idea, the new contribution guide could make for an interesting blog post once it's done.

@mtrezza
Copy link
Member Author

mtrezza commented Dec 29, 2020

This originally larger PR has been broken up into smaller tasks by moving the discussion into issue #7100.
This PR is ready for review.

@mtrezza mtrezza requested a review from a team December 29, 2020 23:18
@mtrezza mtrezza marked this pull request as ready for review December 30, 2020 18:54
@mtrezza mtrezza requested review from davimacedo and dplewis and removed request for a team February 8, 2021 12:34
Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

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

LGTM!

@mtrezza mtrezza merged commit e3ed6e4 into parse-community:master Feb 9, 2021
@mtrezza mtrezza removed the request for review from dplewis February 9, 2021 08:50
mtrezza added a commit to mtrezza/parse-server that referenced this pull request Feb 9, 2021
* commit '7f47b0427ea56214d9b0199f0fcfa4af38794e02':
  Add page localization (parse-community#7128)
  Improve contribution guide (parse-community#7075)
  fix: upgrade pg-promise from 10.9.0 to 10.9.1 (parse-community#7170)
  Add tests against multiple MongoDB versions (parse-community#7161)
  fix: upgrade mime from 2.4.7 to 2.5.0 (parse-community#7166)
  fix: upgrade pg-promise from 10.8.7 to 10.9.0 (parse-community#7168)
  fix: upgrade apollo-server-express from 2.19.1 to 2.19.2 (parse-community#7165)
  Upgrade @node-rs/bcrypt to latest version (parse-community#7159)
  Run Prettier after Definitions (parse-community#7164)
dplewis pushed a commit that referenced this pull request Feb 21, 2021
* add Parse Error guide

* add Parse Server config guide

* removed old instructions for adding config parameters
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Nov 1, 2021
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 14, 2022
@mtrezza mtrezza deleted the improve-contribution-guide branch March 24, 2022 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants