INTERNAL FEEDBACK

[nellshamrell]

Greetings!

The Chef Open Source repo (also known as the "Book of Open Source") is now ready for internal review. The purpose of this repo is to provide guidelines both for how we develop software in an Open Source fashion and how we govern our Open Source projects.

This is still in draft form - nothing is set in stone.

We'd like to collect this feedback in this GitHub issue.

We define feedback as something that is actionable

Please read through the repo and answer the following questions in a comment on this issue:

1) Is there anything you notice that is missing from these documents? 2) How do you feel about the content of these documents? 3) How do you feel about the navigation/flow of these documents? 4) Any other feedback?

To get you started, here are some frequently asked questions:

FAQ

How do we handle private customer information if we are Open Source?

Check out the Project Planning doc.

How do we handle security vulnerabilities if we are Open Source?

Checkout the Security Vulnerabilities doc.

How do we communicate the license differences between the code and binaries in our projects?

Checkout the Required Project Files/Configuration doc under "README.md"

What about the RFC process?

Check out the Design Proposals doc.

How does someone get a contribution license?

Check out the (DRAFT) Contributor Licenses doc.

Has the Product Team been involved in conversations about how to do road map planning, etc.?

YES - we have been working with Phil, Mike, and Morgan on this.

What comes next after this?

Check out our milestones for where this project will go next!

What kind of support are engineers expected to provide?

Checkout the support boundaries doc.

[btm]

1. Is there anything you notice that is missing from these documents?

How do these documents get changed? The README.md refers to the "Chef OSS Practices Committee" being responsible for governance but that is the only place it is mentioned in the repository.

---

https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/design-proposals.md#the-design-proposal-process

Community members and project owners review and discuss the proposal directly in the issue.

2. How do you feel about the content of these documents?

Feels through, happy with that. Anxious about the time and culture change it will take to get the expectations on 'project-membership.md' for smaller projects.

3. How do you feel about the navigation/flow of these documents?

The directories feel odd in structure, but it's not apparent why yet. However, as follow the links in the markdown the markdown at first feels like a nice flow, but then I get lost and fall into using just the directory listings.

Perhaps having an expectation that every file and directory inside a directory must have a section in the README.md for that directory would help. For instance guilds aren't mentioned in the top level README.md.

4. Any other feedback?

these are probably better as separate bugs, but I'll leave them here for now.

---

https://github.com/chef/chef-oss-practices/tree/master/communication#community-slack

The '#general' community Slack channel is the proper place for non-technical discussion and questions.

On first read I thought that meant #random type conversations should happen in #general.

On second read I thought it implied that technical questions should be asked somewhere else.

---

https://github.com/chef/chef-oss-practices/blob/master/contributors/contributor-licenses.md

I was confused here at first thinking this was about Contributor License Agreements/DCO.

Maybe this could be something like 'Software licenses for Contributors'?

---

https://github.com/chef/chef-oss-practices#how-were-organized

The project and/or sub-project scope is defined in each GitHub repository's README.md file.

The project and/or sub-project scope is defined in the README.md file in each individual project's GitHub repository.

---

https://github.com/chef/chef-oss-practices/blob/fd02fc234c4912e096f13594e825949313f10df7/README.md#L41

https://github.com/chef/chef-oss-practices/blob/1a266c3e2d7fddc5bee81071e9e7f59f2b092217/contributors/guide/pull-requests.md#L141

We encourage all contributors to become project members. We aim to grow an active, healthy community of contributors, reviewers, and code owners.

... to invite discussion and feedback from code owners.

project owners?

---

https://github.com/chef/chef-oss-practices/blob/1a266c3e2d7fddc5bee81071e9e7f59f2b092217/contributors/guide/community-expectations.md#code-review

Consequently, as a community we expect that all active participants in the community will also be active reviewers. The community membership outlines the responsibilities of the different contributor roles.

the link on 'community membership' is broken.

[adamhjk]

The phrase “merit” is fraught with historic peril. As used as a principle, I think you could swap it for “benefit”.

It is unclear to me if it’s possible for an outsider to rise in the community to the top levels of leadership and decision making. My guess is the answer is a theoretical yes, but a practical no.

My opinion hasn’t changed on things like aging out contributors and end of life software. In both cases, I prefer a model where individuals resign, rather than are proactively removed due to inactivity. Once a member, you’re a member to you decide the club isn’t for you anymore. Make that process clear.

As for project retirement, I wouldn’t set arbitrary limits. Chef probably has fewer than two people working on all kinds of software that ain’t broke. I think it sets you up to poke people in the eye who don’t need poking. The rest of the world consistently disagrees with me on this point, so y’all do you. But I think it’s unnecessary and perhaps directly harmful to users, and appealing only to the maintainers.

I saw nothing about dispute resolution and final decision making. While rare, you will have times when it is needed. Who can appeal a decision? Who can override one? Is it a committee that decides? What are those rules? Do they vote? Get clear about how power can be used in the model.

That’s my quick feedback. :) it’s a very good start.

[james-stocks]

Automate isn’t mentioned in the opening sentence for https://github.com/chef/chef-oss-practices#how-were-organized - possibly it should be included in the list of Chef products (or if it does not belong in the list, perhaps there should be another sentence in this section explaining the case for Automate).

[echohack]

How do we handle compelled nefarious actions by foreign governments? Should we explicitly call out our processes that prevent this from happening?

What is the process for revoking the contributor license and other contribution rights, especially in the case of nefarious actors? What about hacked (compromised) accounts? How would such an account be restored to good standing?

Case in point:

Australia has passed a new law that can compel Australian employees of companies to install backdoors into software products and compromise the encryption.

See also: https://blog.1password.com/does-australias-access-and-assistance-law-impact-1password/

https://www.google.com/amp/s/www.theverge.com/platform/amp/2018/12/7/18130806/australia-access-and-assistance-encryption-bill-2018-facebook-google-apple-respond

https://www.eff.org/deeplinks/2018/12/new-fight-online-privacy-and-security-australia-falls-what-happens-next

https://signal.org/blog/setback-in-the-outback/

https://www.atlassian.com/blog/announcements/atlassian-urges-changes-to-australian-assistance-and-access-act

[jerryaldrichiii]

@james-stocks I personally think we should just add Automate to that list. Listing all products is...dubious see: https://github.com/chef/chef-oss-practices/issues/17

Would you mind opening an issue or PR so we can track and discuss?

[stevendanna]

Feedback on https://github.com/chef/chef-oss-practices/blob/master/security-vulnerabilities.md

Members of the #security-discuss channel must assign a severity #to the vulnerability, following the latest version of #the CVSS standard.

security-discuss's membership is open to all. Up to this point

it has been filled with people who have independently taken initiative to address security issues across our projects. If we are going to place requirements on the members of that channel, it needs to be accompanied by some sort of organizational support and clearer guidelines of who is responsible for completing these "must"s.

The maintainer fixing the vulnerability should make a private fork of the open source repository.

I'm wondering if we should restrict this to high-severity bugs and otherwise embargoed issues? The vast majority of reported security vulnerabilities are minor and fixing them as part of the normal dev process will prevent unintentional mistakes without meaningfully compromising the security of our users.

[echohack]

I'm wondering if we should restrict this to high-severity bugs and otherwise embargoed issues?

I 100% agree here @stevendanna

The vast majority of security bugs are non-practical vulnerabilities. We do CVE version bump fixes for software ALL THE TIME. Security bugs of this caliber are just bugs, and should follow the same development process as anything else. If the standard development process has problems, then we fix the development process --> don't make special rules just because they are "security" problems.

That said, there are circumstances where critical/high severity practical attacks are discovered and are currently exploitable, with our code. IMO These should go through a critical/high-severity workflow in private and be subject to the industry standard 90 day embargo.

So the workflow should go something like this:

1. Issue is our code, not a simple version bump of external dependency
2. Issue is marked as high/critical security vuln
3. Vuln is determined to be practical/non-theoretical
4. Put this issue in the critical/high-severity practical attack bucket.

The membership of the group of the critical/high-severity practical attacks is what should be restricted, with mandates on extra steps taken to remediate issues.

If I'm wrong about any of this, please feel free to fix me.

[robbkidd]

@btm

(1) How do these documents get changed?

Pull requests to this repository! Upon which discussions will be had and merriment will ensue. Would words to that effect in the How We're Organized section help make that more clear?

(2) Anxious about the time and culture change it will take to get the expectations on 'project-membership.md' for smaller projects.

Open Source Processes Project Seeks Open Source Project(s) Open to New Experiences. Issue #117 is for us to try all this out on a couple of smaller projects at first to learn some things about how all this feels in action.

(3) ... at first feels like a nice flow, but then I get lost and fall into using just the directory listings. Perhaps having an expectation that every file and directory inside a directory must have a section in the README.md for that directory would help.

ETOOMUCHHYPERTEXT? I read this as "If directories are sections, directories should have READMEs and those READMEs should mention their subdirectories/sections." And that makes sense. We'll give this some organizational love.

(various edit suggestions)

Edits made in #160.

[robbkidd]

@adamhjk

The phrase “merit” is fraught with historic peril. As used as a principle, I think you could swap it for “benefit”.

"Merit" replaced in #160

I prefer a model where individuals resign, rather than are proactively removed due to inactivity. Once a member, you’re a member to you decide the club isn’t for you anymore. Make that process clear.

Agreed. @nellshamrell, @tas50 and I concluded that our remaining interest in proactive removal is more about security and least privileges than any sort of social gamification. We'd concluded that contributors can age out due to inactivity from roles like Project Owner and Approver because those roles come with privileged access (repo admin, commit bits, keys to publish artifacts, etc). We think inactive contributors can be demoted (I wish there was a nicer way to phrase that) to Reviewer and remain in the list of "contributors." Resigning completely can be something the individual can choose to do. We can make that more clear.

I saw nothing about dispute resolution and final decision making.

The current idea is that technical dispute resolution final decision making will be performed by the Project Owner of a project within the scope of the project. Community dispute resolution will be handled by an odd-numbered committee of advocates. We can make that more clear.

[robbkidd]

@echohack

How do we handle compelled nefarious actions by foreign governments?

Opened #162 to identify what should be added.

[robbkidd]

@stevendanna @echohack

Opened #163 to refine the security vulnerability handling process.

[msorens]

As an engineer working on Automate 2, I naturally checked out Chef Automate on the short list of Chef projects (https://github.com/chef/chef-oss-practices/blob/master/projects-list.md#chef-software-projects)... Visiting the link takes me to https://github.com/chef/automate/ which led me scratching my head for a moment then I realized: these are not the code(s) you are looking for... I suspect that is the Automate 1 repo. (I never worked on it, so could not say.) Automate 2 is here: https://github.com/chef/a2. (Though I have a vague recollection that this path was going to be renamed for the Open Source effort.)

[tas50]

I'll have to checkup on the effort to rename the repo. I know initially that was going to be part of the OSS effort. A2 is a confusing name externally so I certainly hope we rename the repo.

[stevendanna]

@msorens @tas50 We will be renameing chef/automate -> chef/automate1 (or chef/a1, or /something/) and renaming chef/a2 -> chef/automate. We will likely also be truncating all existing git history.

[mivok]

How do you feel about the navigation/flow of these documents?

This is an initial impression, but I think the main readme is missing a table of contents right at the top. Right now you have the raw files and directories at the top, and a bunch of sections for some specific tasks in a "How do I do X" format for some specific tasks. This is nice if you come here with one of those specific tasks in mind, but having a simple table of contents for the files in the repo at the top would be super helpful for someone coming in for the first time.

Edit: After spending more time reading the files in the repo, I'm even more convinced of the need for a comprehensive table of contents.

[baumanj]

I'll split up my comments into separate issues so people can more easily 👍/👎 them

[baumanj]

Regarding https://github.com/chef/chef-oss-practices/blob/master/governance.md#principles and building on what Adam said

The phrase “merit” is fraught with historic peril. As used as a principle, I think you could swap it for “benefit”.

I love that we're thinking about this, though I think we can do better than just shifting the language because the problem with meritocracy is more than the historical baggage; it's the inherent flaw in the concept: who decides what has benefit/merit? Would any project accept contributions its members didn't think were beneficial?

I think we could drop this bullet altogether. But if we want to transform it into something that captures what I think is the spirit, maybe it should be more about inclusiveness. Isn't what we're trying to say that we strive to not care about where ideas or code came from?

Maybe change

Benefit: Ideas and contributions are accepted according to their technical benefit and alignment with project objectives, scope, and design principles.

to

Inclusiveness: Ideas and contributions are welcome from all those willing to participate in alignment with project objectives. We value the power of different perspectives and enjoy working to understand one another so we can create better solutions than we would separately.

Aaron Turon from the rust leadership team wrote some really interesting things along these lines: http://aturon.github.io/2018/06/02/listening-part-2/

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/governance.md#teams

Teams must have open and transparent proceedings and communication.

How are we going to counter-act the tendency for discussion to occur in the Chef-internal slack? Coworkers need to interact and as long as the public-facing options lose history, people will gravitate toward the private one which doesn't. And once people are using a thing for their normal interaction, important communication is going to happen there almost inevitably.

I'm 100% in favor of putting as much as we can in GitHub issues, but I can't imagine a contributor being well integrated into the day-to-day development of Habitat without seeing the discussions that occur in our Chef-internal slack channel.

[baumanj]

https://github.com/chef/chef-oss-practices/blob/master/governance.md#committees

Do we have any examples of committees?

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/communication/README.md#communication (bold emphasis mine)

Listen. Try to understand someone's position before you negate a suggestion or idea. Ask clarifying questions if something doesn't make sense to you.

I don't think we want to (even implicitly) endorse "negating" someone's position. Instead, let's work to understand it and come to a solution that integrates its concerns, or acknowledge that it's not feasible within our constraints Perhaps:

Listen. Work to understand someone's position and search for better solutions that serve more users. Ask clarifying questions if something doesn't make sense to you.

[baumanj]

https://github.com/chef/chef-oss-practices/blob/master/communication/README.md#get-in-touch

This is a really cool table! What might make it a bit more useful is if some of the if I want to… column were revised it terms of desires that people are likely to have. For instance, instead of "join a Community Slack channel", maybe something oriented to why someone would want to do that like "Have a brief chat about a problem I'm stuck on".

[stevendanna]

Feedback on https://github.com/chef/chef-oss-practices/blob/master/governance.md

Overall, I would like more explanation about how the "teams" talked about in this document relate to our internal development teams. Are they 1-to-1? If so, some words on how we expect the very open teams described in this document to interact with existing engineering management and product planning practices would be very helpful.

Transparent and accessible: Work and collaboration should be done in public. See team governance, below.

At various points, development teams have found it necessary to have private team channels to get work done even on internal Slack. There are well over 20+ internal slack rooms for A2 alone, how do we plan to bridge the gap between our current practices and the proposed practices.

Teams are sets of people within an individual project that focus on various parts of that project. Teams must have open and transparent proceedings and communication. Anyone is welcome to participate and contribute to a team provided they follow the Chef OSS Code of Conduct and read to understand the group's governance policy.

This is a good example of where clearer definition of what "Team" will mean for us going forward would be helpful. Yes, Chef employees can, I suppose, participate in other teams in their off hours, but that would be very different from their expected contributions to the team they are currently assigned to in engineering.

Feedback on https://github.com/chef/chef-oss-practices/blob/master/project-membership.md

Similarly, I think explicit discussion in how this interacts with people's team assignment would be helpful.

Inside engineering, I want to be parts of teams where senior and junior developers are able to participate on equal footing with their relative responsibilities towards code review and approval arrived at as a result of a shared understanding of their relative levels of experience. This kind of understanding changes day-to-day often on fast moving projects.

I am also bit hesitant about the mentions of "activity" in terms of membership requirements. I understand why that is important, but I think we'll need to make clear distinctions about different classes of projects.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/communication/communication-faq.md#why-cant-all-development-discussion-be-in-private-chef-slack

The beauty of Open Source is with so many eyes on the code, the design, etc., it results in better products and a higher impact on the world.

I think that's a bit too simplistic. Many eyes unlock the potential for benefits, but engaging with a community also brings up new challenges. I think it would be good to acknowledge that. Perhaps something like:

The beauty of Open Source is with so many different people and perspectives it can result in better products and a higher impact on the world. This greater potential carries its own challenges as well. The practices defined here are to help us navigate them and achieve our goals.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/communication/communication-faq.md#why-dont-we-just-go-back-to-irc

IRC is extremely unfriendly to anyone without an extensive development/sysadmin/terminal background - our community is made of more than coders and coders of widely different levels of skill.

This still really centers the discussion on coders. Why not:

IRC is isn't as user-friendly as other options - we want our community to be as accessible and inclusive as possible.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/communication/communication-faq.md#why-dont-we-use-discordgittersome-other-chat-tool-instead:

Synchronous chat is difficult for non-native English speakers

I think It's broader than that. It's difficult for anyone who doesn't have the time or resources to keep up. Some people like to think more slowly and carefully. The pace of conversation in a synchronous context is set by the quickest-thinking, quickest-typing users. We should value deliberation and thoughtfulness.

Also I notice "matter" user several places where it sounds odd to me. Is that a usage I'm not familiar with, or should it be "medium"?

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/projects-list.md#chef-software-projects

Should the link be to https://github.com/habitat-sh/habitat/ as it currently is, or https://github.com/habitat-sh/, the organization which includes builder and core plans?

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/README.md#team-structure

Anyone is welcome to jump into a project or engage with a specific team and begin fixing issues, critiquing design proposals, or reviewing code.

Maybe a more neutral term, like, "participating in design discussions"?

Also is reviewing code the kind of contribution we want from a brand new user, or would we prefer the initial engagement to be filing/updating issues or producing new code/documentation so we have some knowledge of them before they start judging the work of others?

[baumanj]

Broken/empty links

• ../../teams/hab-sup/CONTRIBUTING.md in contributors/guide/README.md#find-the-projects-team
• "Security Release Page" in contributors/guide/README.md#security
• "issue triage guide" in contributors/guide/README.md#security
• "here" in contributors/guide/README.md#mentorship
• "Master Teams list" in contributors/guide/contributor-cheatsheet.md#teams
• "Chef Slack channels" in contributors/guide/contributor-cheatsheet.md#community
• "Habitat Dashboard - Development Metrics" in contributors/guide/contributor-cheatsheet.md#other
• "in the automation doc" in contributors/guide/pull-requests.md#automation
• All 3 links in contributors/guide/pull-requests.md#0-familiarize-yourself-with-project-conventions
• "list of teams" in contributors/guide/pull-requests.md#1-is-the-feature-wanted-file-an-enhancement-proposal
• "team" in contributors/guide/pull-requests.md#pull-requests-and-the-release-cycle

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/README.md#contributing

If you find that this is not the case, please complain loudly.

How? To whom?

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/README.md#code-review

It is critical that all code changes to projects be reviewed to promote high quality work and prevent mistakes.

I would argue that the primary point of code review is not to prevent mistakes; it's to foster development norms and keep engaged members apprised of changes so that we keep our bus number up. It's to collaboratively work on the details of a solution with multiple perspectives to give rise to a better solution. And it's to provide immediate feedback about the readability of code that will have to be maintained without the benefit of the author's insight in the future. If we catch mistakes too, I think that's great, but that's more like a fringe benefit than the primary purpose. If we treat code review as mostly about catching mistakes, we encourage drive-by "Looks good to me!" type reviews which aren't very helpful to create and sustain an engaged community.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/community-expectations.md#expectations-of-reviewers-review-comments

Reviewers are highly encouraged to review the code of conduct

This seems too weak. Why not

Reviewers are required to abide by the code of conduct

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/community-expectations.md#expectations-of-reviewers-review-latency

Reviewers are expected to respond to an active PRs with reasonable latency

What's "reasonable"? Should projects be required to have specific SLAs?

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/collaborative-dev.md#code-reviews

Code review increases both the quality and readability of our codebase, which in turn produces high quality software

This is better, but still misses the main point I mentioned in https://github.com/chef/chef-oss-practices/issues/115#issuecomment-470996519. Also, collaborative-dev.md #code-reviews seems to have a lot of redundancy with README.md#code-review. Can we consolidate?

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/collaborative-dev.md#who-performs-a-review

Except for rare cases, such as trivial changes (e.g. typos, comments)

The (perhaps unintended) implication is that comment changes are trivial. Sometimes the are. Sometimes they're not. In some instances comments may even contain executable test code! I think we're better off leaving out the parenthetical and letting people use their judgment about what changes are trivial.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/collaborative-dev.md#who-performs-a-review

project members should not merge their own changes

If they've been appropriately reviewed and approved, why not? The author seems like the best final authority on the merge/no-merge decision.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/collaborative-dev.md#pairing

Chef highly encourages pair programming between its employees and with community members

Does this actually happen? Habitat team members rarely pair with each other, let alone community members.

Should we be pushing for a change in our practices?

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/design-proposals.md#the-design-proposal-process

After a minimum of 2 business days and a maximum of 7 business days the project owners will either close the issue or set the label as accepted.

7 business days seems kind of unrealistic, at least for my team

[baumanj]

https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/design-proposals.md#structure-of-a-design-proposal

This seems like it would benefit from a problem statement. I'm a big fan of the structure described in https://en.wikipedia.org/wiki/Problem_statement#Writing_the_problem_statement:

• Ideal
• Reality
• Consequences
• Proposal

For any design proposal (especially unsolicited), I'd want to know what the problem is and why we should solve it before thinking about a particular solution. Part of developing well for the community is thinking broadly; not just proposing a fix for your particular case. It might be good to even add another sections to list alternative solutions.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/guilds/README.md#recommended-implementation

(ex: #rust-guild in Slack)

Which slack?

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#the-testing-and-merge-workflow

If the failure is a flake, anyone with build system access can simply click retry

Why not make it standard practice to file an issue (perhaps with a well-defined label) if the issue is a flake. And if there's already an issue, comment on it so we can prioritize fixes according to frequency.

Repeat the prior two steps as needed until reviewer(s) add LGTM label

What's wrong with GitHub's approval process? Also, LGTM strikes me as unnecessarily jargon that could make the process less inviting to newcomers.

(Optional) Some reviewers prefer that you squash commits at this step

I know this is a contentious issue, but It would be nice if we could come to some standard (perhaps per-team) here.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#why-is-my-pull-request-not-getting-reviewed

Ping the assignee in Slack.

That seems like it shouldn't be a link to the habitat slack

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#why-is-my-pull-request-not-getting-reviewed

PTAL

I've never heard that one. Seems unnecessarily jargony

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#2-smaller-is-better-small-commits-small-pull-requests

lock in your changes ASAP with your small pull request, and make merges be someone else's problem

Maybe the intended tone is humorous, but I could read as more adversarial than cooperative. Why not

lock in your changes ASAP with your small pull request, and avoid merge headaches

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#3-open-a-different-pull-request-for-fixes-and-generic-features

You absolutely should fix those things (or at least file issues, please) - but not in the same pull request as your feature.

For fixes that are highly local to the areas that the overall PR is changing, I would say that making improvements that aren't strictly necessary in different commits within the same PR should be enough separation. Requiring a separate PR makes the effort level just high enough that I think it discourages iterative improvement.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#4-comments-matter

If you think there's something pretty obvious that we could follow up on, add a TODO.

I think TODOs are to be avoided. It's much more useful to file an issue (and link it in a comment). That's discoverable and prioritizeable and doesn't just clutter the code.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#5-test

Very few pull requests can touch code and NOT touch tests.

This can happen a lot when there isn't good test coverage! Maybe something like:

If the code you're adding doesn't have test coverage, the first step should be determining whether adding it would be valuable and if so adding it.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#6-squashing-and-commit-titles

Each commit should have a good title line (<70 characters) and include an additional description paragraph describing in more detail the change intended.

This is a bit different from the practice suggested in https://chris.beams.io/posts/git-commit/, which https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/collaborative-dev.md#code-reviews links to. I think the arguments to shoot for closer to 50 characters on the title (and wrapping at 72 for the body) are compelling. It might even be worth creating some standard git hooks to help enforce/auto-format this.

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#7-kiss-yagni-mvp-etc

Some links to more detail on these principles would be useful. Also, I'd add DRY

[baumanj]

From https://github.com/chef/chef-oss-practices/blob/master/contributors/guide/pull-requests.md#8-its-ok-to-push-back

You might be overruled, but you might also prevail

This seems like a sort of adversarial framing. How about a more positive-sum way of saying it, like:

Pushing back is often a sign that the reviewer and reviewee have a mismatch of axioms or a mismatch of assumptions. Working to better understand one another can sometimes be challenging, but often leads to better solutions.

[echohack]

Is there a static site (akin to something like this: https://discordapp.com/open-source) where Chef Software Inc. has a list of our major contributions to the projects and software that WE have built OUR software on?

We are already members of rubytogether.org, and already have made major contributions to Ruby, Rust, and many other projects, but where is the one-stop-shop where I can see this?

This kind of resource is necessary to demonstrate to the community that we aren't hypocrites with our open source approach.