Adding a contributing doc by samredai · Pull Request #3414 · apache/iceberg

samredai · 2021-10-29T02:20:16Z

I noticed that the community profile (displayed on the Insights tab) is not all nice and green so I thought we could tackle these items (read more here: About community profiles for public repositories). This PR adds a simple contributing guideline doc, everyone please chime in with things that should be added/removed! :)

As for the Issue template and PR template, I'd like to suggest some variation of these templates created by Steven Mao: https://github.com/stevemao/github-issue-templates

Simple Issues template:

Expected Behavior

Actual Behavior (please include stacktrace if possible)

Steps to Reproduce the Problem

Specifications

Version of Iceberg:
Platform:

Simple Pull Request template:

Fixes #

Proposed Changes

As for the last item, a repo admin should be able to enable reported content from the settings page. Here's more info from the github docs: https://docs.github.com/en/communities/moderating-comments-and-conversations/managing-reported-content-in-your-organizations-repository

RussellSpitzer · 2021-11-02T15:38:32Z

I would prefer the pull request template have some of the info from the issue template as sometimes folks just go straight to the PR. I also think it's a bit harder to keep the two in sync. I think Spark's template is pretty good

<!--
Thanks for sending a pull request!  Here are some tips for you:
  1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
  2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
  3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
  4. Be sure to keep the PR description updated to reflect all changes.
  5. Please write your PR title to summarize what this PR proposes.
  6. If possible, provide a concise example to reproduce the issue for a faster review.
  7. If you want to add a new configuration, please read the guideline first for naming configurations in
     'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
  8. If you want to add or modify an error type or message, please read the guideline first in
     'core/src/main/resources/error/README.md'.
-->

### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
  1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
  2. If you fix some SQL features, you can provide some references of other DBMSes.
  3. If there is design documentation, please add the link.
  4. If there is a discussion in the mailing list, please add the link.
-->


### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
  1. If you propose a new API, clarify the use case for a new API.
  2. If you fix a bug, you can clarify why it is a bug.
-->


### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->


### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->

samredai · 2021-11-10T18:49:51Z

@rdblue I realized that since this is in the root directory it's outside of the ignore path for all of the github workflows . This is just adding a markdown file to the repo root though so it's safe to merge without all tests passing after review+approval.

rdblue · 2021-11-14T21:32:26Z

+
+## Pull Request Process
+
+Pull requests are the preferred mechanism for contributing to Iceberg. PRs will be automatically labeled based on the content by our github-actions labeling bot. Although it is not required, if the PR is related to an issue, it's helpful to prefix your PR title with the issue number surrounded by brackets, for example `[1234] <PR title>`. Another common practice is to prefix the PR title with `[WIP]` if it's a PR that you opened for visibility and may not necessarily be ready for review or merging.


Actually, we don't use issue references or WIP like this. A WIP PR should be marked as a draft. And for related issues, the description should state "Closes #1234" instead of using brackets in the description. That will automatically close the issue, and takes less of the limited summary space.

We also find it helpful if people include a prefix like "Build:", "Docs:", "Spark:", "Flink:", "Core:", "API:", etc. to give reviewers context.

Updated this to:

Although it is not required, if the PR is related to an issue, it's
helpful to prefix your PR title with the issue number surrounded by brackets, for example Closes #1234 <PR title>.
With this format of issue reference in the PR title, the issue will be automatically closed when the PR is merged.
It's also helpful to include additional prefixes that help provide context to PR reviewers, such as Build:, Docs:,
Spark:, Flink:, Core:, API:. Another thing to keep in mind is that if it's a PR that you've opened for
visibility and may not necessarily be ready for review or merging, be sure to convert the PR to a draft.

We actually don't want people putting the issue in the title. That makes issue lists harder to read. Instead, the "Closes #1234" should be in the issue description.

rdblue · 2021-11-14T21:32:37Z

+
+Pull requests are the preferred mechanism for contributing to Iceberg. PRs will be automatically labeled based on the content by our github-actions labeling bot. Although it is not required, if the PR is related to an issue, it's helpful to prefix your PR title with the issue number surrounded by brackets, for example `[1234] <PR title>`. Another common practice is to prefix the PR title with `[WIP]` if it's a PR that you opened for visibility and may not necessarily be ready for review or merging.
+
+## Styling


rdblue · 2021-11-14T21:33:06Z

+
+## Styling
+
+For Java styling, check out the section [Setting up IDE and Code Style](https://iceberg.apache.org/community/#setting-up-ide-and-code-style) from the documentation site. For Python, please use the tox command `tox -e format` to apply autoformatting to the project.


This is a good start. Once this is committed, I'll add some style guidelines that we follow as well.

rdblue · 2021-11-14T21:33:27Z

+
+For Java styling, check out the section [Setting up IDE and Code Style](https://iceberg.apache.org/community/#setting-up-ide-and-code-style) from the documentation site. For Python, please use the tox command `tox -e format` to apply autoformatting to the project.
+
+## Building the Project Locally


Might want to move this above Style, since the style section is probably going to get a lot longer.

RussellSpitzer · 2021-11-17T20:08:00Z

+
+If you are thinking of contributing but first would like to discuss the change you wish to make, we welcome you to
+head over to the [Community](https://iceberg.apache.org/community/) page on the official Iceberg documentation site
+to find a number of ways to connect with the community, including slack and our mailing lists. Of course, always feel


I would just add hyperlinks for "slack" and "mailing lists" to just make this a little easier

RussellSpitzer · 2021-11-17T20:09:19Z

+
+Pull requests are the preferred mechanism for contributing to Iceberg. PRs will be automatically labeled based on
+the content by our github-actions labeling bot. Although it is not required, if the PR is related to an issue, it's
+helpful to prefix your PR title with the issue number surrounded by brackets, for example `Closes #1234 <PR title>`.


I thought you add this part in a comment? If not I"ll do this from now on :) I'm used to our prs just being

Component: Issue Title

RussellSpitzer

Added a few suggestions, but I think this is a great start. I think we should get these drafts up and online ASAP and do fine tuning later.

rdblue · 2021-11-17T23:15:34Z

As Russell suggests, I'm going to merge this to get it out there. We can fine-tune it later.

Adding a contributing doc

a884733

Adding Apache License info

e448cf0

rdblue reviewed Nov 14, 2021

View reviewed changes

Updating with PR comments

aa0e3ef

RussellSpitzer reviewed Nov 17, 2021

View reviewed changes

RussellSpitzer approved these changes Nov 17, 2021

View reviewed changes

Update CONTRIBUTING.md

e054859

rdblue merged commit f7206c6 into apache:master Nov 17, 2021

Initial-neko pushed a commit to Initial-neko/iceberg that referenced this pull request Nov 23, 2021

Docs: Add CONTRIBUTING.md (apache#3414)

ed3bf68


		## Pull Request Process

		Pull requests are the preferred mechanism for contributing to Iceberg. PRs will be automatically labeled based on the content by our github-actions labeling bot. Although it is not required, if the PR is related to an issue, it's helpful to prefix your PR title with the issue number surrounded by brackets, for example `[1234] <PR title>`. Another common practice is to prefix the PR title with `[WIP]` if it's a PR that you opened for visibility and may not necessarily be ready for review or merging.


		Pull requests are the preferred mechanism for contributing to Iceberg. PRs will be automatically labeled based on the content by our github-actions labeling bot. Although it is not required, if the PR is related to an issue, it's helpful to prefix your PR title with the issue number surrounded by brackets, for example `[1234] <PR title>`. Another common practice is to prefix the PR title with `[WIP]` if it's a PR that you opened for visibility and may not necessarily be ready for review or merging.

		## Styling


		## Styling

		For Java styling, check out the section [Setting up IDE and Code Style](https://iceberg.apache.org/community/#setting-up-ide-and-code-style) from the documentation site. For Python, please use the tox command `tox -e format` to apply autoformatting to the project.


		For Java styling, check out the section [Setting up IDE and Code Style](https://iceberg.apache.org/community/#setting-up-ide-and-code-style) from the documentation site. For Python, please use the tox command `tox -e format` to apply autoformatting to the project.

		## Building the Project Locally

Uh oh!

Conversation

samredai commented Oct 29, 2021

Simple Issues template:

Expected Behavior

Actual Behavior (please include stacktrace if possible)

Steps to Reproduce the Problem

Specifications

Simple Pull Request template:

Proposed Changes

Uh oh!

RussellSpitzer commented Nov 2, 2021

Uh oh!

samredai commented Nov 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer left a comment

Choose a reason for hiding this comment

Uh oh!

rdblue commented Nov 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

samredai commented Nov 10, 2021 •

edited

Loading