artsy / README

:wave: - The documentation for being an Artsy Engineer
Creative Commons Attribution 4.0 International
1.1k stars 120 forks source link

RFC: Updating Best Practices Documentation #492

Closed patrinoua closed 1 year ago

patrinoua commented 1 year ago

Proposal

It is proposed that the team maintains an up-to-date document containing best practices for various aspects of our work. This document should be regularly reviewed and updated to ensure that it reflects current best (preferred) practices and industry standards.

Reasoning

Having an up-to-date best practices document is beneficial for several reasons:

Exceptions

There may be situations where it is not practical or possible to update the best practices document in a timely manner. In such cases, the team should consider the following exceptions:

Additional Context

In a big team it can be hard to stay up to date with latest decisions as not all stakeholders are part of the decision making process. Rather than retroactively seeking help on the several dev-help channels maintaining a doc as a single source of truth can help with keeping everyone up to date. It can spark discussions and make sure decisions happen sooner (on the decision level) rather than later (on the PR level).

How is this RFC resolved?

This RFC will be considered resolved when the team has established an agreement towards regularly updating high level documentation (README / best_practices), as part of our process. This agreement could include some of the following steps:

damassi commented 1 year ago

One suggestion could be to simplify some of the process described in the resolution and establish a single primary outcome. Something like your point above

To establish a standard such as this across all PDDE teams would be a great win! Something like "documentation day/week", etc. And then the additional bullets can exist as a flexible suggestions for teams on how to execute the primary outcome. I fear that having each of these bullet points existing side-by-side might be a somewhat difficult bar to reach from a team-wide perspective (this could also just be my personal preference for less process speaking) where a nested primary-secondary relationship could be the best of both worlds.

mzikherman commented 1 year ago

I think this is a great idea, but feel that it'll suffer from a lot of other documentation-related efforts. It becomes another thing that gets out of date 😅 and won't be updated regularly after the first push.

At some point, the Artsy development space and repos and everything is so broad that it's truly difficult to stay up to date on every repo's best practice, flat out. At some point, if you are working on a new (to you) repo, or you're a newer developer to the team, you will likely need to ask questions, flail around a bit, pair with someone. When working on implementing something, you'd need to check-in often with your approach to make sure it's up to convention. Don't develop a new feature all on your own only to discover at PR time it's not following a lot of conventions. That's what earlier PR's, iterative commits and even a mini-tech-plan might be for. I'm just not super hopeful there will be a magical document that will exist to help this.

Nevertheless I support this effort/idea and hope to be proven wrong!

erikdstock commented 1 year ago

I wanted to write exactly what @mzikherman said. I'd love to see it work, and maybe some of what @damassi suggests about regular review could help - but i think we often want to believe that practices will naturally flow from documentation, when what we really have is a common organizational problem that is a product of teams that span different technical areas, microservices, levels of experience and languages. In my experience this gets us the worst in services that are touched the least often by multiple teams.

Shared documentation adds another abstraction to be maintained by multiple teams without the guarantee its mandates will make it back into the code, including prioritizing finding and eliminating tech debt.

I don't mean for this to sound negative and think it would be great to solve this problem though! Some ideas that might be helpful or not:

mzikherman commented 1 year ago

designating people assigned to track each repo's conformance, including reviewing as many PRs as possible

I like this idea and have been trying (inconsistently) to do it for Gravity and Metaphysics.

patrinoua commented 1 year ago

I feel like everyone likes the idea but we're a bit afraid of the implementation.

Fear not!

One simple solution would be: Just update stuff. Don't think too much about it. Add information on the repo's best_practices. Not everything. We don't need to make sure we keep all of our 200+ repositories up to date! Only the stuff we actually use every day.

Is there something you recently decided on that more than 2 people should know about?

Did you just spend half an hour or two hours figuring things out?

Just add a quick note.

I think we just need to talk about it and increase team's awareness and willingness to this. And the common understanding that it can be really useful to everybody.

Imagine a scenario where you are a part of a team with 5-6 developers and you have a discussion with one other team member and you start implementing something. However you and the other person did not have the latest information! In big teams this can happen some times! So you go ahead implement and open a PR about it! And then you get a lot of feedback on the PR level to change things.

If you or your team members had gone ahead and updated best_practices with simple notes such as "we use this" or "we really want to avoid doing this" there could have been an early alignment about this and it could have saved everybody time, frustration and double work!

Take another example: A repo becomes outdated and is replaced by some other repo. Would be lovely to have it in mind to update this on the README for both repos! Real life example: I recently tried to figure out what is the current situation of palette , palette-storybook and icons. All these repositories export front end components and icons. Their readme were out of date, there was no reference from one to the other, and I had no idea what I should choose.

A simple note there (2 minutes?) would be sufficient to save the frustration and feeling overwhelmed by too many options and outdated information, and not being sure if you're using the right thing.

Yes, we all learn the systems we work with by time!

But let us also keep in mind the Future Developer.

What this RFC is trying to say with really simple words is:

Again, we do not have to spend too much time polishing everything!

erikdstock commented 1 year ago

Some thoughts came to me today:

mdole commented 1 year ago

In general, I support having better documentation that helps people self-start when working in new areas/repos/teams. I'd like to get more specific with what an outcome of this RFC would look like.

IMO there are two main problems with docs:

  1. Engineer could not find the docs (or didn't think to look for them since they weren't obvious)
  2. Docs are out of date

We actually have a lot of docs! But it's not always clear where to find something - and of course, sometimes they can also easily become out-of-date.

As written, the RFC proposes a single document with best practices. Some of the other folks who have commented have also hinted at more repo-specific documents (like READMEs) to explain the current state of those repos. I would say we need both - a "look here for stuff that isn't repo-specific" doc/set of docs, and helpful docs in our major repos.

I would suggest we:

Thanks for kicking off this discussion @patrinoua!

erikdstock commented 1 year ago

Designate one future friday this quarter for an engineering-wide "doc bash" where we assign repos out to pairs of participants (hopefully having frequent contributors contribute to repos they're familiar with) to spend some time standardizing & making updates

Agree with your post except for this @mdole because I would like to pilot this right now in pulse. Is this something that we can adopt incrementally/as needed?

Edit: I think I can answer my own question and say there's no reason a single repository can't manage itself internally, so your comment didn't block this.

joeyAghion commented 1 year ago

Some of what's described above is very explicitly laid out in our documentation playbook. It describes what should go in Github vs. Notion, README expectations, and public vs. private criteria. (If there are significant gaps to that document, please propose updates.)

There is already regular (though organic) investment in these forms of documentation. On any particular topic, I'd guess that we have a stale document more often than we have no document, so before adopting any new conventions (or new locations to look for docs!) I'd want us to apply our existing conventions more consistently.

Of course, adding a best practices doc to repos that could benefit is very compatible with our conventions. I'd just suggest doing that in response to specific gaps, and always linking from the other READMEs or playbooks.

patrinoua commented 1 year ago

Thank you all for the discussion! I'm going to close this for now as something between 4: Acceptance, with Little Feedback. / 5: Unclear Resolution. Anyone that would like to look more into this and how we could best keep our best practices up to date, feel free.