add code of conduct and community guidelines #32

anarcat · 2017-02-09T23:33:41Z

Okay, here it goes... The objective here is to set clear guidelines on how to participate and contribute to this project. This is to ensure that what happened to linkchecker (the project was abandoned) doesn't happen again. It also ensures that we have a great project that is fun to participate in, without any problematic behavior that could ruin the work of our volunteers. It also tries to set examples on how to contribute on various levels - bug triage, patches and so on. It formalizes the review process we have experimented with.

More importantly, at this stage, it sets guidelines on how to share responsibility on the project. Right now, I have made a few people "members" here in a ad-hoc decision, but I have tried to formalize this process here as well.

Following the process defined here, at least one other member would need to approve this pull request before it is merged, at which point it would become policy. :)

This partly resolves #1. You'll notice there's a bunch of stuff missing, but there's only so much time and I want to get the ball rolling for the ownership, which seems to still be a critical SPOF with me in the middle. :)

anarcat · 2017-02-09T23:37:46Z

to see a rendered version, go at https://github.com/linkcheck/linkchecker/blob/a5dc2d861c73ade8412c8ad59033d60db73a99f0/doc/contributing.mdwn

maybe we'd need a table of contents?

what we really need is some online rendering of all this documentation, which should be merged with the website... so much to do!

ghost · 2017-02-10T09:20:10Z

I think this is generally a good idea though it doesn't get round the dependence problem - need more active people! Also, I think fundamental decisions about 'the constitution' should be an exception to the rule about just needing one person to approve; it's important that everyone agrees.

mgedmin

👍 💯 except I think the two copies of the covenant in two files were an accident you'll want to fix before merging

mgedmin · 2017-02-10T12:48:36Z

doc/contributing.mdwn

+[pull requests]: https://github.com/linkcheck/linkchecker/pulls
+[Github project]: https://github.com/linkcheck/linkchecker
+
+Some guidelines for patches:


This sentence repeats the last sentence of the previous paragraph. Seems redundant.

mgedmin · 2017-02-10T12:49:24Z

doc/contributing.mdwn

+* A patch should be a minimal and accurate answer to exactly one
+  identified and agreed problem.
+* A patch must compile cleanly and pass project self-tests on at least
+  the principle target platform.


s/principle/principal/, I think

actually, all supported platforms must pass (regardless of what those are) so i just changed that...

mgedmin · 2017-02-10T12:53:31Z

doc/contributing.mdwn

+
+Maintainers should not merge their own patches unless there is no
+response from other maintainers within a reasonable time frame (1-2
+days).


I suggest rephrasing to "Maintainers should not merge their own patches if they haven't been approved by other maintainers, unless ...".

I believe usually (for values of "usually" that apply to a project set up a week ago ;) we do this:

maintainer A submits a pull request

maintainer B reviews

maintainer A hits the merge button on their own pull request

Also 1-2 days seems short -- I for one don't plan to be reviewing pull requests on weekends. "About a week" seems more reasonable, unless the patch is urgent.

agreed, I tried to rephrase.

mgedmin · 2017-02-10T13:17:44Z

doc/contributing.mdwn

+# Patches
+
+Patches can be submitted through [pull requests][] on the
+[Github project][]. Some guidelines for patches:


I think the correct capitalization is GitHub.

fixed everywhere in that document...

mgedmin · 2017-02-10T13:19:03Z

doc/contributing.mdwn

+   should be refused for other reasons, use the `wontfix` label and
+   close the issue
+-  mark feature requests with the `enhancement` label, bugs with
+   `bug`, duplicates with `duplicate` and so on...


It may be worth mentioning that only project members are allowed to label or close issues.

tried to clarify.

A patch commit message must consist of a single short (less than 50 characters) line stating the a summary of the change, followed by a blank line and then a description of the problem being solved and its solution, or a reason for the change. Write more information, not less, in the commit log.

'the a' -> a

The 'commit log' is /doc/changelog.txt, in linkchecker. Maybe worth mentioning explicitly as it's not immediately obvious there is one. There isn't one in linkchecker-gui, needs creating.

fixed the typo. i think we should keep changelog.txt to broader changes than just every commit log entry - those are separate things... changelog.txt is part of the release process, IMHO, which we need to improve, in doc/development.mdwn...

mgedmin · 2017-02-10T13:20:06Z

doc/contributing.mdwn

+There are three levels of membership in Github organizations, Owner,
+Member, or regular user. Anyone is welcome to contribute to the
+project within the guidelines outlined in this document, regardless of
+their status, and that includes regular users.


It is unclear to me if "regular users" are everyone with a GitHub account, or only those that have been invited into the linkcheck organization.

tried to clarify.

mgedmin · 2017-02-10T13:21:39Z

doc/development.mdwn

+On Mac OS X systems, using MacPorts, Fink or homebrew for software
+installation is recommended.
+
+- Install Python >= 2.7.2 from http://www.python.org/


Seems a rather low version. I think you need at least 2.7.6 for proper TLS support (e.g. SNI)?

Also, can we make all the download URLs https://, provided they work?

i did not actually change this - just renamed the file...

mgedmin · 2017-02-10T13:23:37Z

doc/development.mdwn

+  or for Windows from http://www.sosdg.org/clamav-win32/
+
+- *Optional, for displaying country codes:*
+  Pygeoip from http://code.google.com/p/pygeoip/


The url says "This project has been moved over to github and will be deleted soon: https://github.com/appliedsec/pygeoip"

But anyway https://pypi.python.org/pypi/pygeoip exists, so maybe we should just add it to our install_requires and/or requirements.txt and drop the mention here?

it seems you are reviewing the whole development.txt file - which i just renamed, i didn't actually change it that much... maybe we can split off those suggestions in a different PR? :)

Hey, free review for text I wouldn't have othervise reviewed ;)

mgedmin · 2017-02-10T13:25:43Z

doc/development.mdwn

+
+2. bump AppVersion in setup.py and commit changes
+
+3. build wheel file (setup.py bdist_wheel) and upload to PyPI


in other projects, i keep source files on github because sdist files are not reproducible from the git tree directly, so they are different from github. so i keep only binary files on PyPI... but i'm fine with changing that - anyways, we don't have access to the PyPI archive (yet?) so this is mostly theoritical ... (#4)

fixed, anyways.

mgedmin · 2017-02-10T13:26:20Z

doc/development.mdwn

+
+6. write release notes on github
+
+# Contributor Covenant Code of Conduct


Do we want this repeated twice in two files?

definitely not, fixed. :)

…sensus among the owners

tough call, but i think it actually matters... In the code of conduct, a "community member" is not necessarily an official status. by conflating the "maintainer" and "member" status, we would have diminished the role of non-official members... People are a member of the community when they participate, regardless of their official status, and deserve the same respect as anyone else. also use "administrator" instead of "owner". i do not "own" this project, i administer it. we do not have to comply with GitHub's proprietary vocabulary

ghost · 2017-02-10T14:15:38Z

A patch must compile cleanly and pass project self-tests on all target platforms.

Do we refuse pull requests from Unix users who haven't tested on Windows and vice versa? Or does this mean that our target platforms have to be limited to what Travis supports? I don't think a 'must' here is practical.

anarcat · 2017-02-10T14:15:47Z

okay, pushed a bunch more changes. @seamang i added an explicit rule about changing the constitution, that it requires all "adminstrators" (which i use instead of "owners" now, see 74487ae for the rationale) to have consensus.

i have also tried to address @mgedmin's excellent review. sorry for the mess about the duplicate stuff. also note that i didn't really change the development.txt file, i just renamed it to .mdwn which is why it looks like a new file, but it's just copy-paste, really. although it's true the release process is my own stuff, which is why i agree to change it here, it was changed in a previous (unreviewed!) push...

i agree we need more active people, but i think this will come with time... hopefully this document will help rather than be a nuisance. ;)

@seamang please do review the PR as well. ;)

anarcat · 2017-02-10T14:17:19Z

Do we refuse pull requests from Unix users who haven't tested on Windows and vice versa? Or does this mean that our target platforms have to be limited to what Travis supports? I don't think a 'must' here is practical.

Well, we just need to clarify somewhere what our target platforms are. I don't think the statement should be interpreted as "we refuse code that wasn't tested on all platforms", but more as "we refuse code that fails to pass tests on all platforms", which is quite different. that we don't have a Windows test platform doesn't force (IMHO) UNIX users to install Windows 95 to test their shit. ;)

mgedmin · 2017-02-10T15:15:40Z

Requiring every contributer to test on all supported platforms is not practical; that's why services like Travis CI (for Linux and Mac OS) and Appveyor exist.

We don't currently have Appveyor (or Mac OS targets for Travis CI) set up, but we should have, if we intend to support those platforms. I can help with appveyor.yml, but I've never touched a Mac, so I hope somebody else steps up!

mgedmin

Eh, LGTM.

mgedmin · 2017-02-10T15:18:18Z

doc/contributing.mdwn

+
+There are three levels of membership in the project, Administrator
+(also known as "Owner" in GitHub), Maintainer (also known as "Member")
+, or regular users (everyone with or without a GitHub account). Anyone


The newline in front of a comma seems weird to me.

I'm still tempted to nitpick about "membership in the project". I think there are two levels of membership, but an important point is that you don't have to be a member to contribute.

fixed the comma.

i welcome modifications on the wording of course. ;)

anarcat · 2017-02-10T15:44:25Z

Requiring every contributer to test on all supported platforms is not practical; that's why services like Travis CI (for Linux and Mac OS) and Appveyor exist.

that's right.

We don't currently have Appveyor (or Mac OS targets for Travis CI) set up, but we should have, if we intend to support those platforms. I can help with appveyor.yml, but I've never touched a Mac, so I hope somebody else steps up!

I thought travis had mac support? https://docs.travis-ci.com/user/multi-os/

the problem is really windows, and that's why we need Appveyor, AFAIK. i'm open to all of those. :)

anarcat · 2017-02-10T15:44:47Z

@seamang i think we only need your final approval before we go forward here.

ghost

Good to go - we can tinker with this in the future if needed.

anarcat · 2017-02-10T17:05:59Z

awesome, thanks everyone for your reviews and feedback!

anarcat · 2017-02-10T17:09:03Z

oh, and i forgot to ask for another person to be added to the CoC contact list - see #33 for followup.

mgedmin · 2017-02-13T15:18:23Z

I thought travis had mac support? https://docs.travis-ci.com/user/multi-os/

Yes, but we don't have it set up in our .travis.yml at the moment. If you want an example of how other projects use Travis CI to build Mac OS X wheels for various Python versions, you can take a look at https://github.com/zopefoundation/zope.interface/blob/master/.travis.yml#L19-L33

anarcat · 2017-02-13T16:20:24Z

sounds like another PR waiting to be born. ;)

mgedmin · 2017-02-13T16:43:19Z

We could use a volunteer Mac maintainer.

I might be convinced to step up, if somebody wants to ship me a Macbook.

mgedmin · 2017-02-14T12:23:18Z

I might be convinced to step up, if somebody wants to ship me a Macbook.

This sounds as if I'm serious. To clarify: I'm not. ;)

anarcat · 2017-02-14T15:16:35Z

maybe that could just be a differnt issue? :)

anarcat added 3 commits February 9, 2017 17:50

fix file extension for development file, it's markdown

bcacd8b

import Covenant's Code of Conduct 1.4

9d2cec5

adapt the code of conduct to our community, add myself as contact

a2fce98

anarcat requested review from mgedmin and a user February 9, 2017 23:33

anarcat mentioned this pull request Feb 9, 2017

contribution guidelines and FAQ #1

Open

6 tasks

mgedmin requested changes Feb 10, 2017

View reviewed changes

move code of conduct to a separate document, add other guidelines

c84621e

anarcat force-pushed the coc-refactor branch from a5dc2d8 to c84621e Compare February 10, 2017 13:51

anarcat added 13 commits February 10, 2017 08:52

also build sdist files

a9cb943

clarify who regular users are

fad1b72

clarify that labeling is privileged

7162371

rephrase and expand timeframe

c8dab7d

clarify that all supported platforms should test

92cbe36

remove duplicate sentence

8ed6c0c

s/Github/GitHub/

b20b294

remove another duplicate sentence

f07b581

remove double-negative

7731f09

allow urgent self-merges

d097567

explicitely state that all PRs must be reviewed before merge

1102510

add exception that changes to the contribution document must have con…

df82d8c

…sensus among the owners

mgedmin approved these changes Feb 10, 2017

View reviewed changes

fix weird line ending issue

ede76dc

ghost approved these changes Feb 10, 2017

View reviewed changes

fix typo

f56f6a3

anarcat merged commit bf66006 into master Feb 10, 2017

anarcat deleted the coc-refactor branch February 10, 2017 17:06


		2. bump AppVersion in setup.py and commit changes

		3. build wheel file (setup.py bdist_wheel) and upload to PyPI


		6. write release notes on github

		# Contributor Covenant Code of Conduct

add code of conduct and community guidelines #32

add code of conduct and community guidelines #32

Conversation

anarcat commented Feb 9, 2017

anarcat commented Feb 9, 2017

ghost commented Feb 10, 2017

mgedmin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Feb 10, 2017

anarcat commented Feb 10, 2017

anarcat commented Feb 10, 2017

mgedmin commented Feb 10, 2017 • edited Loading

mgedmin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anarcat commented Feb 10, 2017

anarcat commented Feb 10, 2017

ghost left a comment

Choose a reason for hiding this comment

anarcat commented Feb 10, 2017

anarcat commented Feb 10, 2017

mgedmin commented Feb 13, 2017

anarcat commented Feb 13, 2017

mgedmin commented Feb 13, 2017

mgedmin commented Feb 14, 2017

anarcat commented Feb 14, 2017

mgedmin commented Feb 10, 2017 •

edited

Loading