Skip to content

Criteria/signoffs for merging PRs #52

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

Open
SethTisue opened this issue Dec 7, 2020 · 6 comments
Open

Criteria/signoffs for merging PRs #52

SethTisue opened this issue Dec 7, 2020 · 6 comments

Comments

@SethTisue
Copy link
Member

SethTisue commented Dec 7, 2020

We now have several PRs that seem mergeable.

But I think we should probably hold off on actually merging any PRs here until we have some kind of signoff process. It probably doesn't need to be super formal, but this repo is still pretty new and I don't want standard library additions flying in under people's radar. Where “people” means: the Scala 2 team at Lightbend; the Dotty team at EPFL; key folks at the Scala Center such as @julienrf; and the community.

Off the top of my head: how about a label that indicates that something is considered mergeable? Then every so often, we can publicize that a batch of PRs is ready for hopefully-final review/signoff.

We use to have SLIP for this, but I don't suggest we try to bring that back. The basics I suggest we need are:

  • Scala 2 team discusses at their team meeting, if needed.
  • Dotty team discusses at their team meeting, if needed.
  • There is sufficiently-publicized opportunity for community input.

I think most of the time there will be consensus or near-consensus. If some particular change ends up being sufficiently controversial that it's not clear what to do, well, I guess we can cross that bridge when we come to it :-)

@julienrf
Copy link
Contributor

a label that indicates that something is considered mergeable

Who would set this label?

@julienrf
Copy link
Contributor

Should we also provide a template to write issues, and require people to maintain the issue description up to date with what has been possibly discussed, so that reviewers can easily refer to the corresponding issue of a PR to understand better the addressed problem, why it is important, etc.?

@som-snytt
Copy link

If the bar is high for merging ("it represents the final version of future API, everyone agrees") then there will be minimal activity.

It's difficult to see beneath the amazingly ambitious and prolific contributions by Scala Steward.

Maybe what's needed is an artifact with a low bar for merging, scala-library-experimental or scala-library-visionary, which people would be using now so that the many ideas in the tickets can be vetted through experience.

Only efforts that survive that process would be promoted to -next status, and this artifact could guarantee trivial migration in Scala 3.3 or whenever that happens.

@julienrf
Copy link
Contributor

julienrf commented Aug 9, 2022

Scala 3 provides a way to define “experimental” APIs. Those APIs don’t provide binary compatibility guarantees. I believe we should leverage that mechanism to ship APIs to the end-users without having to be 100% sure they are stable.

However, we would still need a sort of “compat” library for cross-building (like scala-collection-compat).

@NthPortal
Copy link
Contributor

honestly, I think there are several reasons this repo never went anywhere:

  • not enough interest in contributing to it
  • significant limitations in what features can be added via implicit faffing
    • any future of this library is necessarily significantly inferior to a theoretical "scala-library" % "2.14.x"
    • it would be better and easier to just work on a 2.14.x version of the standard library
  • general disinterest from the core team and/or the community in developing a 2.14.x version of the standard library

@som-snytt
Copy link

We could just rename it scala-collection-incompat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants