chore: initial practices readme #2086

AlastairHolmes · 2022-08-25T13:46:15Z

Just an outline at this point, I added some possible headings. I've tried to make what we discussed more concrete, and clear. I really wanna make sure we are all on the same page, and this is written clearer. So even small issues please comment on them.

msgmaxim · 2022-08-26T02:01:44Z

PRACTICES.md

@@ -0,0 +1,51 @@
+# Justification
+
+The rust development team as a whole has identified problems in our development process. This document is intended to help us avoid these problems.


When someone reads this document in the future, I don't think they necessarily need to know that "we had issues". (Not that we need to keep it secret, but I just don't think it is necessary and feels a bit less professional)

I think it is better to state it in a spirit of "To minimise arguments during code review and ensure code consistency, all (rust) developers must follow the principles outlined below" (as an example, don't need to be these exact words).

Ok, I agree we don't need to talk about that we had problems. But I think the problem isn't really frequent disagreements, but the fact that those disagreements go no where / have no conclusion?

msgmaxim · 2022-08-26T02:09:32Z

PRACTICES.md

+
+The rust development team as a whole has identified problems in our development process. This document is intended to help us avoid these problems.
+
+The core of the problems we identified are frequent disagreements, regarding both code and development operations, that do not conclude or result in agreement. This lead to frustration among the development team which caused a lack of dialog between developers, distrust among the team members, and developers wasting significant time disagreeing without making progressing on problems. Regularly issues or pull requests would be paralzed due to an extended heated discussion about a detail which did not result in agreement. Instead everyone ended up being entrenched and so discussion stopped as it felt no progress towards a conclusion or agreement could be made.


Again, it is okay elaborate on "the things we want to avoid", but I would state this more generally as "these things tend to happen when there are no guidelines". I don't mind if we briefly mention that this is based on previous experience at CF, but right now it feels like someone wrote this out of frustration (understandably).

Agreed, let's formulate this in positive terms, ie. "having clear guidelines helps resolve disagreements, fosters collaboration, and facilitates onboarding of new team members".

I am happy to make this more positive, but I think it is important to be highly specific, otherwise people could totally miss the point. If we all understand the symptoms, then we are likely to notice them and do the right thing.

To be clear, I did not write this in frustration. I was approximately as calm as I can be.

PRACTICES.md

kylezs · 2022-08-26T09:03:13Z

PRACTICES.md

+
+# Maintainence
+
+Simply create an issue on this repository regarding the modification to / problem with the practices, provide a justification for the change, and add the <span style="color:rgb(0,255,0)">practices</span> label. We intentionally chose not to not burden ourselves with a formal process to agree on modifications to the practices, but it is expected the whole team be given the opportinity to comment and a majority of the team should agree to a change. Once the modification is agreed on by the team, create a pull request to modify this document. That pull request should contain changes that ensure the codebase matches the new practices if applicable.


On the "majority of the team should agree" I think it would be nice to say something like, "if this addition to the document has come from a disagreement between two or more parties, it's important that all of these parties agree to the commitment added to the document."
Else it's easy to see a scenario where the one person who felt the other way is alienated - there might be some cases where majority rules, but I think in the ideal scenario, there's buy in from those directly involved, in the form of either a compromise, or one or the other has been convinced and we decide to commit to a particular way of doing things (assuming others also agree to this).

Do we want to say "majority" or "all" are satisfied?

I think majority is fine for now, hesitant to say "all", would be quite a slow process...

kylezs · 2022-08-26T09:03:56Z

PRACTICES.md

+
+Simply create an issue on this repository regarding the modification to / problem with the practices, provide a justification for the change, and add the <span style="color:rgb(0,255,0)">practices</span> label. We intentionally chose not to not burden ourselves with a formal process to agree on modifications to the practices, but it is expected the whole team be given the opportinity to comment and a majority of the team should agree to a change. Once the modification is agreed on by the team, create a pull request to modify this document. That pull request should contain changes that ensure the codebase matches the new practices if applicable.
+
+When documenting a practice you should write a clear explanation with a justification, and if at all possible some brief examples of its application. As without examples it is easy to miss-interpret the intention of a particular practice.


Suggested change

When documenting a practice you should write a clear explanation with a justification, and if at all possible some brief examples of its application. As without examples it is easy to miss-interpret the intention of a particular practice.

When documenting a practice you should write a clear explanation with a justification, and if at all possible some brief examples of its application (ideally from our codebase - can link to PRs or issues). As without examples it is easy to miss-interpret the intention of a particular practice.

PRACTICES.md

dandanlen

Some early comments, will re-read next week.

dandanlen · 2022-08-26T15:50:32Z

PRACTICES.md

+# Location
+
+We choose to document our practices here inside this repository, so they are easily assessible and we can use Github's source control tools to manage the maintainence of this document.
+


Not a strong opinion but I don't think this is necessary, the location is obvious and the reasons should be obvious from the methods proposed in the remainder of the doc (ie. PRs).

I'd rather air on the side of clarity. We had a discussion about it, and a choice was made. I think that should be recorded and clear in the future. I'm sure there are developer's that think the guidelines should be elsewhere.

dandanlen · 2022-08-26T15:55:09Z

PRACTICES.md

+- Clearly explained, as if they are ambiguous that will allow room for disagreement to persist.
+- Well-reasoned otherwise we likely alienate some developers, as such each practice should be justified using logic, principles, ideals, and other practices.


Also, providing the reasoning allows for that reasoning to be questioned and for things to be refined when exceptions arise, or cases where the reasoning doesn't apply.

(That's not very concise..)

dandanlen · 2022-08-26T15:55:33Z

PRACTICES.md

+
+Simply create an issue on this repository regarding the modification to / problem with the practices, provide a justification for the change, and add the <span style="color:rgb(0,255,0)">practices</span> label. We intentionally chose not to not burden ourselves with a formal process to agree on modifications to the practices, but it is expected the whole team be given the opportinity to comment and a majority of the team should agree to a change. Once the modification is agreed on by the team, create a pull request to modify this document. That pull request should contain changes that ensure the codebase matches the new practices if applicable.
+
+When documenting a practice you should write a clear explanation with a justification, and if at all possible some brief examples of its application. As without examples it is easy to miss-interpret the intention of a particular practice.


Thought: we could provide some kind of template for changes?

You mean a GitHub issue template? Or you mean a template in this file? Either way I can create one.

I personally find these things pointless, and that they are a crutch (For not properly explaining yourself).

PRACTICES.md

Co-authored-by: kylezs <kyle@chainflip.io>

Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com>

AlastairHolmes · 2022-09-01T09:30:35Z

PRACTICES.md

+# Justification
+
+The intention of this document is to ensure the team makes decisions, work can move forward, and the team members are comfortable with those decisions, even when the members initially do not agree.
+


I kind of gave up trying to write some more explanation here. I could not find a nice concise explanation for why we want this document. @kylezs If you feel it would be helpful it might be nice you tried to write something.

How about:

All members of the team should have a shared view on what "good" code should look like. This document serves as a guide to current and future contributors on some of the more subjective aspects of programming. This ought to reduce uncertainty among contributors about how their code should look, leading to faster contributions to the main code base, since reviewers and reviewees spend less time on the more subjective details of "style".

* PRACTICES Readme * Update PRACTICES.md Co-authored-by: kylezs <kyle@chainflip.io> * Update PRACTICES.md Co-authored-by: kylezs <kyle@chainflip.io> * Update PRACTICES.md Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com> * chore: review changes Co-authored-by: kylezs <kyle@chainflip.io> Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com>

* develop: Chore/refactor remove witness (#2127) Refactor: Changed participants from`Vec` to `BTreeSet` (#2116) chore: initial practices readme (#2086)

PRACTICES Readme

bb81b3a

AlastairHolmes requested review from dandanlen, kylezs and msgmaxim August 25, 2022 13:46

AlastairHolmes added the practices Pertaining to the development practices in the PRACTICES.md file label Aug 25, 2022

AlastairHolmes changed the title ~~chore: Initial practices Readme~~ chore: initial practices readme Aug 25, 2022

AlastairHolmes mentioned this pull request Aug 25, 2022

Develop Development Practices #2087

Closed

msgmaxim reviewed Aug 26, 2022

View reviewed changes

kylezs reviewed Aug 26, 2022

View reviewed changes

dandanlen reviewed Aug 26, 2022

View reviewed changes

AlastairHolmes and others added 4 commits August 29, 2022 15:51

Update PRACTICES.md

d91cf51

Co-authored-by: kylezs <kyle@chainflip.io>

Update PRACTICES.md

39f3280

Co-authored-by: kylezs <kyle@chainflip.io>

Update PRACTICES.md

6db9a32

Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com>

chore: review changes

0010d6c

AlastairHolmes commented Sep 1, 2022

View reviewed changes

AlastairHolmes requested review from msgmaxim, dandanlen and kylezs September 1, 2022 09:30

kylezs approved these changes Sep 5, 2022

View reviewed changes

msgmaxim approved these changes Sep 6, 2022

View reviewed changes

dandanlen approved these changes Sep 6, 2022

View reviewed changes

AlastairHolmes merged commit 0a5e8f6 into develop Sep 6, 2022

AlastairHolmes deleted the chore/initial-practices-readme branch September 6, 2022 08:01

syan095 added a commit that referenced this pull request Sep 7, 2022

Merge branch 'develop' into fix/purge-witnesser-data

246cb55

* develop: Chore/refactor remove witness (#2127) Refactor: Changed participants from`Vec` to `BTreeSet` (#2116) chore: initial practices readme (#2086)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: initial practices readme #2086

chore: initial practices readme #2086

AlastairHolmes commented Aug 25, 2022 •

edited

Loading

msgmaxim Aug 26, 2022

AlastairHolmes Aug 29, 2022

msgmaxim Aug 26, 2022

dandanlen Aug 26, 2022

AlastairHolmes Aug 29, 2022

AlastairHolmes Aug 29, 2022

kylezs Aug 26, 2022

AlastairHolmes Aug 31, 2022 •

edited

Loading

kylezs Aug 31, 2022

kylezs Aug 26, 2022

dandanlen left a comment

dandanlen Aug 26, 2022

AlastairHolmes Aug 29, 2022

dandanlen Aug 26, 2022

dandanlen Aug 26, 2022

AlastairHolmes Aug 29, 2022 •

edited

Loading

AlastairHolmes Sep 1, 2022

kylezs Sep 1, 2022 •

edited

Loading

		@@ -0,0 +1,51 @@
		# Justification

		The rust development team as a whole has identified problems in our development process. This document is intended to help us avoid these problems.


		The rust development team as a whole has identified problems in our development process. This document is intended to help us avoid these problems.

		The core of the problems we identified are frequent disagreements, regarding both code and development operations, that do not conclude or result in agreement. This lead to frustration among the development team which caused a lack of dialog between developers, distrust among the team members, and developers wasting significant time disagreeing without making progressing on problems. Regularly issues or pull requests would be paralzed due to an extended heated discussion about a detail which did not result in agreement. Instead everyone ended up being entrenched and so discussion stopped as it felt no progress towards a conclusion or agreement could be made.


		# Maintainence

		Simply create an issue on this repository regarding the modification to / problem with the practices, provide a justification for the change, and add the <span style="color:rgb(0,255,0)">practices</span> label. We intentionally chose not to not burden ourselves with a formal process to agree on modifications to the practices, but it is expected the whole team be given the opportinity to comment and a majority of the team should agree to a change. Once the modification is agreed on by the team, create a pull request to modify this document. That pull request should contain changes that ensure the codebase matches the new practices if applicable.


		Simply create an issue on this repository regarding the modification to / problem with the practices, provide a justification for the change, and add the <span style="color:rgb(0,255,0)">practices</span> label. We intentionally chose not to not burden ourselves with a formal process to agree on modifications to the practices, but it is expected the whole team be given the opportinity to comment and a majority of the team should agree to a change. Once the modification is agreed on by the team, create a pull request to modify this document. That pull request should contain changes that ensure the codebase matches the new practices if applicable.

		When documenting a practice you should write a clear explanation with a justification, and if at all possible some brief examples of its application. As without examples it is easy to miss-interpret the intention of a particular practice.

		# Location

		We choose to document our practices here inside this repository, so they are easily assessible and we can use Github's source control tools to manage the maintainence of this document.

		- Clearly explained, as if they are ambiguous that will allow room for disagreement to persist.
		- Well-reasoned otherwise we likely alienate some developers, as such each practice should be justified using logic, principles, ideals, and other practices.

		# Justification

		The intention of this document is to ensure the team makes decisions, work can move forward, and the team members are comfortable with those decisions, even when the members initially do not agree.

chore: initial practices readme #2086

chore: initial practices readme #2086

Conversation

AlastairHolmes commented Aug 25, 2022 • edited Loading

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

AlastairHolmes Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dandanlen 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

AlastairHolmes Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylezs Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

AlastairHolmes commented Aug 25, 2022 •

edited

Loading

AlastairHolmes Aug 31, 2022 •

edited

Loading

AlastairHolmes Aug 29, 2022 •

edited

Loading

kylezs Sep 1, 2022 •

edited

Loading