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

Adding contributing guide and contributors list. #570

Merged
merged 6 commits into from May 22, 2019
Merged

Adding contributing guide and contributors list. #570

merged 6 commits into from May 22, 2019

Conversation

lewisd32
Copy link
Contributor

@lewisd32 lewisd32 commented May 16, 2019

No description provided.

CONTRIBUTING.adoc Outdated Show resolved Hide resolved
* The OS/platform and version
* Contextual information (e.g. what you were trying to achieve with ServiceTalk)
* Simplest possible steps to reproduce
** The more complex the steps are, lower the priority it will likely be.
Copy link
Member

@idelpivnitskiy idelpivnitskiy May 16, 2019

Choose a reason for hiding this comment

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

I'm not sure this is a right statement. Even for complex steps bugs could be critical.

Copy link
Contributor Author

@lewisd32 lewisd32 May 21, 2019

Choose a reason for hiding this comment

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

I borrowed and rephrased this sentence from here.
The intent was to encourage simple reproduction steps. If it's complicated/complex to reproduce, then I think it is less likely to be a priority as something with simpler reproduction steps, given equal severity.
Would you suggest changing this sentence somehow, or removing it?

Copy link
Member

@idelpivnitskiy idelpivnitskiy May 22, 2019

Choose a reason for hiding this comment

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

I will suggest just to remove it. It is confusing. We can add something to clarity this item later if we have a better idea.

CONTRIBUTING.adoc Outdated Show resolved Hide resolved
CONTRIBUTING.adoc Show resolved Hide resolved
CONTRIBUTORS.txt Outdated Show resolved Hide resolved

Please be sure to include:

* The ServiceTalk version
Copy link
Member

@idelpivnitskiy idelpivnitskiy May 16, 2019

Choose a reason for hiding this comment

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

We probably should add a debug/info log statement with ST version to help users understand which ST version is in classpath.

Copy link
Contributor Author

@lewisd32 lewisd32 May 21, 2019

Choose a reason for hiding this comment

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

Good idea. #578

CONTRIBUTORS.txt Outdated Show resolved Hide resolved
.mailmap Outdated Show resolved Hide resolved
* The OS/platform and version
* Contextual information (e.g. what you were trying to achieve with ServiceTalk)
* Simplest possible steps to reproduce
** The more complex the steps are, lower the priority it will likely be.
Copy link
Member

@idelpivnitskiy idelpivnitskiy May 22, 2019

Choose a reason for hiding this comment

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

I will suggest just to remove it. It is confusing. We can add something to clarity this item later if we have a better idea.

Please use the following checklist before pushing your commits or issuing a pull request:

- Does your work build without any failure when you run `./gradlew build` from shell?
- Does your work introduce any new IDEA inspector warnings?
Copy link
Member

@idelpivnitskiy idelpivnitskiy May 22, 2019

Choose a reason for hiding this comment

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

What if our contributor is an Eclipse user? I feel like this item is not important and could be removed for now. Anyway, we are not strict with it in our team and we don't have a unified configuration for inspections that we can share with contributors. Netty has it, that's why they ask about inspections in checklist: https://netty.io/wiki/setting-up-development-environment.html#inspection-profile

If we want to keep it, consider to clarify that you are talking about IntelliJ IDEA (maybe also good to have a like to https://www.jetbrains.com/help/idea/code-inspection.html, but not necessary).

Copy link
Member

@ddossot ddossot May 22, 2019

Choose a reason for hiding this comment

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

It is currently impossible to open the project in Eclipse, so I think it's good to make it clear that IDEA is needed.

Copy link
Contributor Author

@lewisd32 lewisd32 May 22, 2019

Choose a reason for hiding this comment

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

Removed

CONTRIBUTING.adoc Outdated Show resolved Hide resolved
CONTRIBUTING.adoc Show resolved Hide resolved
CONTRIBUTING.adoc Outdated Show resolved Hide resolved
CONTRIBUTING.adoc Outdated Show resolved Hide resolved
CONTRIBUTING.adoc Outdated Show resolved Hide resolved
@lewisd32 lewisd32 merged commit 4320d42 into apple:master May 22, 2019
0 of 2 checks passed
@lewisd32 lewisd32 deleted the add-contributing-guide branch May 22, 2019
@apple apple deleted a comment from lewisd32 Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants