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

feat: introduce proposal design process #3333

Merged
merged 11 commits into from
Nov 11, 2022
Merged

feat: introduce proposal design process #3333

merged 11 commits into from
Nov 11, 2022

Conversation

masci
Copy link
Contributor

@masci masci commented Oct 6, 2022

Related Issues

  • fixes n/a

Proposed Changes:

  • Introduce a formal process to detail substantial changes to Haystack
  • Migrate one old similar doc (ADR approach) to the new process

Notes for the reviewer

ADR feels limited for our use case:

  • it only captures a decision and the reasons behind that.
  • it's written AFTER something has already been decided and/or implemented

A proposal on the other side:

  • it is written BEFORE even adding code
  • it contains plenty of details about why it is needed but more importantly how it would be implemented

Checklist

@masci masci requested review from a team as code owners October 6, 2022 08:52
@masci masci requested review from ZanSara and removed request for a team October 6, 2022 08:52
rfcs/README.md Outdated Show resolved Hide resolved
@masci masci requested review from TuanaCelik and removed request for ZanSara October 6, 2022 09:35
@masci masci changed the title Massi/rfc feat: introduce RFC process Oct 6, 2022
rfcs/0000-template.md Outdated Show resolved Hide resolved
rfcs/README.md Outdated Show resolved Hide resolved
rfcs/README.md Outdated Show resolved Hide resolved
@vblagoje
Copy link
Member

vblagoje commented Oct 7, 2022

LGTM, looking forward to using it! @masci I can open the RFC template in logseq and work on it just like any other markdown document I already write in logseq. This is awesome!
Screenshot 2022-10-07 at 14 37 48

rfcs/0000-template.md Outdated Show resolved Hide resolved
rfcs/0000-template.md Outdated Show resolved Hide resolved
rfcs/0000-template.md Outdated Show resolved Hide resolved
rfcs/0000-template.md Outdated Show resolved Hide resolved
rfcs/0000-template.md Outdated Show resolved Hide resolved
rfcs/README.md Outdated Show resolved Hide resolved
rfcs/README.md Outdated Show resolved Hide resolved
rfcs/README.md Outdated Show resolved Hide resolved
rfcs/README.md Outdated Show resolved Hide resolved
rfcs/text/2170-pydantic-dataclasses.md Outdated Show resolved Hide resolved
@julian-risch
Copy link
Member

One remark from my side is about the internal use of ADRs in other teams (link to private repo). It would be good if we could keep the RFC process and the ADRs aligned as closely as possible. Let's not have different processes across teams if it's avoidable. As the RFC process will also influence other teams, I would suggest asking them for feedback as well. Getting diverse opinions. 🙂
@masci Maybe you could add a brief explanation why to replace ADRs with RFCs in the PR description? And if you would like to get more feedback tag a few people from other teams? Otherwise the PR looks good to me. 👍

@bogdankostic
Copy link
Contributor

I agree with Julian that we should explain why we are replacing ADRs with RFCs, other than that, this looks really good to me :)

@masci
Copy link
Contributor Author

masci commented Oct 7, 2022

@julian-risch @bogdankostic ADR and RFC are not the same! Also, as much as I love consistency, different teams require different tools, specially our team that shares the project with a community of 100+ external contributors.

Also worth mentioning ADR were never really adopted in Haystack (there was only one) so we're not really "replacing" an old process, think about this PR as introducing one.

rfcs/README.md Outdated Show resolved Hide resolved
@masci masci added the type:documentation Improvements on the docs label Oct 11, 2022
rfcs/README.md Outdated Show resolved Hide resolved
@vblagoje
Copy link
Member

What do we need to do to integrate this PR? I am excited to issue the first RFC, which I am sure @ZanSara wants to improve with her ideas for multimodality and more. Let's kick off this process @masci

@masci
Copy link
Contributor Author

masci commented Oct 25, 2022

@vblagoje waiting for @tholor to provide feedback on the initiative at large.

In the meantime, can you open an issue using the same template in this PR, without mentioning the RFC process so we can start discussing your proposal?

@tholor
Copy link
Member

tholor commented Oct 25, 2022

@masci @vblagoje Yep, would like to wait for some feedback from the team survey this week, before officially launching this process and sharing it with the community.

Of course, this shouldn't stop you from using the template here already to share and discuss a proposal @vblagoje.

masci and others added 8 commits November 9, 2022 18:59
Co-authored-by: Agnieszka Marzec <97166305+agnieszka-m@users.noreply.github.com>
Co-authored-by: Agnieszka Marzec <97166305+agnieszka-m@users.noreply.github.com>
@masci masci changed the title feat: introduce RFC process feat: introduce design process Nov 9, 2022
Copy link
Member

@tholor tholor left a comment

Choose a reason for hiding this comment

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

Good to go from my side!

Sorry for holding this one up, but I felt it was worth understanding what we want to improve with it and what the biggest pain points are right now. The team survey + conversations definitely helped with that.

So, let's experiment with it and see if helps us to improve documentation, speed and clarity of decisions. I could also see that we do a few small iterations on the exact format / process after we made some learnings.

@tholor tholor changed the title feat: introduce design process feat: introduce proposal design process Nov 10, 2022
Copy link
Member

@vblagoje vblagoje left a comment

Choose a reason for hiding this comment

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

LGTM

proposals/text/2170-pydantic-dataclasses.md Outdated Show resolved Hide resolved
@masci masci merged commit da6b0dc into main Nov 11, 2022
@masci masci deleted the massi/rfc branch November 11, 2022 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.