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

fix: update contribution doc to be more welcoming to new comers #2514

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

0x2f2f2f2f
Copy link

@0x2f2f2f2f 0x2f2f2f2f commented Apr 9, 2024

Purpose of the PR

update documentation to be more similar to https://github.com/GreptimeTeam/greptimedb/contribute when it comes to first contributions to make it more welcoming for new contributers.

Main Changes

Modifying documentation to be easier for new comers to understand the codebase and better understand set up of repository.

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (please modify tests here).
  • Need tests and can be verified as follows:
    • xxx

Does this PR potentially affect the following parts?

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. feature New feature labels Apr 9, 2024
@0x2f2f2f2f 0x2f2f2f2f changed the title [Improvement]: udpate contribution doc to be more welcoming to new comers fix: update contribution doc to be more welcoming to new comers Apr 9, 2024
@0x2f2f2f2f 0x2f2f2f2f marked this pull request as draft April 9, 2024 15:00
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.75%. Comparing base (1228dec) to head (f794390).
Report is 1 commits behind head on master.

❗ Current head f794390 differs from pull request most recent head 94d74bc. Consider uploading reports for the commit 94d74bc to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2514      +/-   ##
============================================
- Coverage     61.03%   52.75%   -8.28%     
+ Complexity      827      488     -339     
============================================
  Files           579      579              
  Lines         46232    46232              
  Branches       6275     6275              
============================================
- Hits          28219    24392    -3827     
- Misses        15198    19275    +4077     
+ Partials       2815     2565     -250     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@0x2f2f2f2f 0x2f2f2f2f requested a review from imbajin April 12, 2024 13:54
@imbajin imbajin requested a review from VGalaxies April 14, 2024 16:06
@imbajin imbajin marked this pull request as ready for review April 14, 2024 16:06
Copy link
Contributor

@VGalaxies VGalaxies left a comment

Choose a reason for hiding this comment

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

@imbajin Is it necessary to strictly enforce hard wrap (most diffs are caused by this)?


[![contributors graph](https://contrib.rocks/image?repo=apache/hugegraph)](https://github.com/apache/incubator-hugegraph/graphs/contributors)
Pull Requests
Copy link
Contributor

@VGalaxies VGalaxies Apr 15, 2024

Choose a reason for hiding this comment

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

Alone on its own line?

@imbajin
Copy link
Member

imbajin commented Apr 15, 2024

@imbajin Is it necessary to strictly enforce hard wrap (most diffs are caused by this)?

Currently nope (This is happening due to we lack the max-chars control for *.md files 😿 & IDEA used 100 for default)

But it's hard for new comer/devs to know the background, so the auto wrap occurred now.

Maybe we should update the config(max-length) in hugegraph-style.xml early?

@VGalaxies
Copy link
Contributor

@imbajin Is it necessary to strictly enforce hard wrap (most diffs are caused by this)?

Currently nope (This is happening due to we lack the max-chars control for *.md files 😿 & IDEA used 100 for default)

But it's hard for new comer/devs to know the background, so the auto wrap occurred now.

Maybe we should update the config(max-length) in hugegraph-style.xml early?

@returnToInnocence could take a look here~

@returnToInnocence
Copy link
Contributor

returnToInnocence commented Apr 16, 2024

@returnToInnocence could take a look here~

#2525 The problem of adapting the maximum length of markdown files can be solved with this pr

@0x2f2f2f2f u could update with the master branch & revert the markdown line break changes in this pr (restart IDEA if not work)

@apache apache deleted a comment from returnToInnocence Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants