PR pileup

[panesofglass]

The list of pull requests seems quite long, with several PRs numbering in the thousands of lines changed. Can someone work on getting these worked into various branches or closed with at least a "Thanks for your contribution, but now's not the right time" response? I am aware a lot of energy is going into the core CLR support, and the community agrees that is very important. However, I fear this repo is in danger of dissuading OSS contributions due to perceived futility. It is no fun trying to keep a PR up-to-date with a moving target, and it is no fun trying to merge stale PRs.

Several possible directions for moving forward, most mentioned elsewhere:

1. Split VS tooling, the compiler, and other parts to make it easier to maintain each repo.
2. @KevinRansom has mentioned to me many times that MS has to handle certain things, e.g. Core CLR, templates, etc. If this is the case, then perhaps we should focus external contributions on https://gihtub.com/fsharp/fsharp with a clear migration path between that repo and this one.
3. Create a community branch and assign community members to merge PRs into that branch and keep it consistent with master or another, MS maintained branch so that we are at least keeping things moving.
4. Create a Kanban-style goal of a maximum number of allowed open PRs and commit to merging or closing them daily or weekly.
5. Treat all branches as equals. Core CLR is very important, but it should not shut down all work in the repository or it becomes a terminal disease. Each feature branch should merge in master until it is ready to merge into the main branch. That may delay important features a bit, but I think that's worth the effort.
6. I'm sure other good ideas will avail themselves.

Projects get a lot of excitement by managing their contributions well. Rust does this well. @forki is regularly praised on Twitter for his amazing success with PRs and issues. Please make visualfsharp a success, as well.

NOTE: I wasn't entirely sure where to post this, so I'm going with an issue here.

[CraigStuntz]

Agreed, having to keep a PR open and working for months is frustrating!

[enricosada]

👍 some pr are ready, these should go in or closed.

But the close reason should not be "there is no time for merge/testing", that's deadly for community.

If a pr it's a good feature/bugfix, it's reviewed, should go in, in master branch.

Having only a master branch, it's better because everyone try the same branch. Who try coreclr branch? we are only delaying issues to next merge. And i need to build multiple times to test the compiler.
If the problem it's the fear of bugs before rtm, let's use a stabilization branch (like roslyn) or a relase branch before rtm. If now the coreclr it's the developmente branch, all pr should target it.

1. about split, see Split FSharp.Core in a separate repository #861 or Extract type providers to standalone projects #441 , it's really a big repo, let's make it easier to manage it.
2. about two repositories ( fsharp/fsharp and visualfsharp ) please dont do it. Not if they are the same codebase, It's tons of time wasted on merge ( see Integrate Symbol API etc. of FSharp.Compiler.Service into Visual F# Tools #853 from fcs, or the multiple merge into fsharp/fsharp ). From visualfsharp -> fsharp it's easy, @dsyme and community can help ( they are merging already from visualfsharp ). The problem from fsharp -> visualfsharp it's the same problem as having pr to merge, but worse because merge are going to be bigger. Lot bigger. If @dsyme say that visualfsharp it's the gold repository to contribute to compiler/fsharpcore/etc, we should make pr there. fsharp/fsharp it's only packaging. If the policy change, ok, but having two released compiler/fsharpcore versions (community and ms ) with different features/fixes it's a bad idea, really bad. The gold repository should be only one. Same for FSharp.Core nuget. We are really going to have two different nuget packages for coreclr/desktop and mono/etc ( ref Split FSharp.Core in a separate repository #861) ?
3. community branch = unstable = it's the master branch. I dont understand how that can help if the release it's from ms. What we do if ms doesnt want to merge a commit from community branch? that's more work, who can be done with review inside a pr.
4. promise to close/merge x pr each week it's good.
5. i think coreclr branch should be merged inside master. There is no need for a different branch. It's the developmente branch. For example i want to add responsefiles, i need to wait merge in master, and sync from master -> coreclr. Now coreclr branch it's stable, move it inside master branch, the codebase it's going to be merged anyway. Less time wasted on merge, more time for review pr.

[panesofglass]

@enricosada, I agree. I just thought I would throw out some options for the sake of argument. You have fulfilled my hopes. 💃 Also, I think a stabilization branch is a good move.

[dsyme]

@panesofglass I get what you're saying. If you have specific work you are doing which is affected by PR backlog please let us know? @enricosada is an active contributor to the repo and makes some important points.

Equally, it's very important that we don't put pressure on repo owners to merge anything that might risk regressions or be destabilizing - or at least to fully evaluate the cost/benefits. I do feel that some "please merge" pressure was part of what led us to do inadequate code reviews on an optimization PR for Visual Studio 2015 update 1, which led to a regression. I don't want to see further regressions like that, and I'd prefer a PR pile up (ultimately resolved by the contributors) than regressions. If necessary, PRs can stay open for as long as needed until we're 100% comfortable with merging them.

[isaacabraham]

@dsyme that's a fair point. Is there a strategy that can be put in place to alleviate the effort to keep PRs up to date though? A PR staying alive for months means that typically the contributor has to ensure that it keeps up to date with the latest master lest it be disregarded later on for being too old (something we have seen happen before).

I'm not saying the team should be accepting PRs willy nilly or feeling pressure from the community - that's not the right option either - but is there some way to meet in the middle?

[dsyme]

@isaacabraham I understand what you're saying.

If the repo owners deem a PR is unsuitable for an update release, then it will necessarily be delayed until the branch is ready for the next major revision. (There should probably be a process where PRs can be brought up-to-date by people apart from the submitter, though GitHub doesn't make that particularly easy since the PR has to be resubmitted).

One way we can get PRs in is to open "next major version" branches. However branch management takes significant time and can itself be error prone and a cause of fragmentation in the repo - again making it harder to contribute. Until a "next major revision" branch is open, it is better to keep the PR open and incrementally update it. This may be likely to happen with any major language work, or major optimizations. It's in the nature of the beast.

I do think splitting the repos into IDE tooling and Compiler would be helpful. I'm less concerned about splitting out FSharp.Core though that could also be done.

I don't like seeing any PRs closed because they are "too old" unless they truly bring no value and should be closed anyway, or have truly diverged to the point that they can't be rescued. If you see examples of this please from the past please link to them.

We all want F# to both progress, evolve and yet be stable and reliable. There are many, many ways to contribute to core F# engineering and tooling outside this specific repo (e.g. in the many IDE tooling projects). But generating excitement via a high rate of PRs is not a goal in and of itself - only where it achieves other goals.

[isaacabraham]

Perhaps one thing could be for a "quick review" up front by someone in the team and at least provisionally give a go-ahead in principle for acceptance of a PR. In this way, if a PR is unsuitable because e.g. feature not needed / implementation or whatever - it can be closed off quickly. On the other hand, ones with value can then be grouped into a bucket for review - and a process can be put in place to deal with them (somehow).

This is somewhat akin to how you deal with UV feature requests - a quick review to identify ones of value in principle, and then a more detailed review later on.

Not saying that that is a perfect solution, but maybe a start?

[isaacabraham]

Regarding the "too old" comment - I thought that some of the work on unit tests was held off for a long time in the run up to F#4 and then eventually rejected as out of date => risk of merge too high. I might be completely wrong here though.

[dsyme]

@isaacabraham I think that may eventually have been merged. But if not I suspect the repo owner may have been more worried about churn, scale of change and so on. Please send a link if possible.

The same could well happen with any piece of work. My experience making contributions to different projects over the years is that the larger the piece of work the longer it takes to be integrated, the harder it is to find a suitable place to integrate it, and the more at risk it is of eventually being bumped.

[dsyme]

Perhaps one thing could be for a "quick review" up front by someone in the team

I think that happens already.

[panesofglass]

Thanks for your reply, @dsyme. I don't have any recent contributions waiting. I submitted a PR a long time ago for the web project templates when the fsharp source was on Codeplex. I was told someone would move them over and that a lot of work was required on the MS side and that I would be unable to help much more. I've never seen those merged or even brought over into this repo. I know templates are wanted, but I have no motivation to do anything more given what I know about the template contribution process (in that it doesn't appear to exist).

I raised this issue b/c 1) I was made aware of the large number of outstanding PRs and 2) I have seen some frustration and heard comments indicating that future contributions were going to be forfeited. That's a really sad plight for an OSS project, and I hoped to identify some way to help move things along.

Another option I did not yet propose would be someone maintaining a community fork that would assimilate community contributions into a single, ever-growing PR for the owners of this repo to review and merge. This option puts the onus of verifying contributions on the community first, thereby hopefully cutting the time required by the owners of this repo. At present, this would represent both a massive effort and a massive PR, which I agree would be very difficult to merge. The bright side is that once this is done, the community could provide a single stream of community contributions for review by this repository. I suggested fsharp/fsharp above b/c that would at least provide a more direct line to releasing testable alphas on NuGet. However, since it is not a fork of this repo, moving contributions back becomes more difficult (unless something has changed).

[panesofglass]

Also, fixing the testing situation could help address the problem of regressions by allowing us to add more tests more easily (if it's not already fixed, that is).

[enricosada]

@dsyme nobody is asking for bad quality pr, or fast track merge, but i think 3 weeks is enough time for multiple reviews and feedback. If after three weeks a pr is not ready, should be dropped.

I think more people should contribute to review, that's a really important part.
That mean repo owner too, if possibile not only @KevinRansom but also @NumberByColors and @otawfik-ms .

Everytime i wrote better code (bug, cleanup, perf) in my prs with review feedback from you @dsyme, @KevinRansom and community.
And i learned a lot too.

If a pr is too big that can be split in multiple smaller pr, easier to review, and let some time pass between each.
For example i have one pr for in memory compilation, that's a refactoring only, with minimized diff. After that pr is merged i'll continue with another with the api for in memory compilation, and another with files in memory like source files.

I understand everything can break (i wrote a regression for binary compatibility with f# 4 last time, so i understand), but having a more fluid release process and beta, can help a lot.
If v4.1.0 break something, the v4.1.1 can fix it. it's better is we can do that fast, like having a separate fsharp.core nuget package => faster release than compiler. The full packaged release inside visualstudio can release the latest nuget.

There were some occasion with single line diff pr, to fix compilation, with more than > 1 week before merge.
That's very annoying for contributors

But that doesnt mean we should lower the bar for quality, the opposite, more review and smaller commit.

I am happy ms maintains this repo, but it should be easier to contribute. It's a lot easier with roslyn or dotnetcli ( my experience at least ).
the real issue i see a lot of time is wasted on merge, instead of make easier to contribute to this repo.
The community can help a lot, not all features are coreclr/top priority, but everyone want to improve different areas, that's the real 👍 ihmo, if guided/reviews by msteam and @dsyme . Make it easier to do it

[panesofglass]

👍, @enricosada. We can always leverage the power of git cherry-pick -x ... to pull in specific commits for a stable branch, too.

[dsyme]

I have ... heard comments indicating that future contributions were going to be forfeited

That is incorrect, so you can ignore it.

[panesofglass]

@dsyme I meant the potential contributors stated they were considering no longer submitting or working on contributions. Apologies for the unclear wording.

[dsyme]

@panesofglass OK, thanks. I think the best way to help is to do more code review, and to help keep PRs up-to-date. Also bring over your previous submissions from CodePlex please!

[KevinRansom]

It's not clear that I can help you out with this. The bulk of the commits are Don's.and so he is the most inconvenienced by this, and it doesn't seem to have slowed him down any. There are a couple from Enrico, which are in the nice to have rather than commited feature, etc.

What feature are you working on @panesofglass that requires one or more of these PR's and which PR does your feature need?

Kevin

[7sharp9]

One thing that makes it difficult for me is I work on OSX, to to make prime commits that will be accepted and I have to PR here. But ... I have to work in fsharp/fsharp to use and test my patch, cherry pick to FCS to test tool integration, then port to visualfsharp to make the PR so that the whole lot can be back merged into FCS and fsharp/fsharp so that it can be publicly consumed. That's quite painful!!

On 19 Jan 2016, at 23:31, Kevin Ransom (msft) [email protected] wrote:

It's not clear that I can help you out with this. The bulk of the commits are Don's.and so he is the most inconvenienced by this, and it doesn't seem to have slowed him down any. There are a couple from Enrico, which are in the nice to have rather than commited feature, etc.

What feature are you working on @panesofglass that requires one or more of these PR's and which PR does your feature need?

Kevin

—
Reply to this email directly or view it on GitHub.

[KevinRansom]

@7sharp9 being able to develop on OSX and Linux is definitely a goal, and we are tiptoing towards that. However right now it is not yet doable. But it will eventually.

[panesofglass]

@KevinRansom, I'm not working on anything related to this repo currently. I had hoped to help out with some things previously but was unable to build any branch for quite some time. Only today did I discover that master builds on my VM.

I'm not sure of the state of the web templates PRs. I assume they are out-of-date (targeting VS2013) and lost somewhere on CodePlex.

[dsyme]

@panesofglass Your PR to codeplex is here. Please bring it across, rebase, and submit to github? If you could, that would be great, thanks:

[enricosada]

@KevinRansom :

@7sharp9 being able to develop on OSX and Linux is definitely a goal, and we are tiptoing towards that. However right now it is not yet doable. But it will eventually.

How? can we create an issue with problems and roadmap about osx/linux? I am pretty sure exists already.
Can we help?
For example, my next pr can be:

• import build script from fsharp/fsharp? use an add appveyor-build.sh, like .cmd but bash (like dotnet/cli does, duplicated script)
• make fsprojs and solution work with monodevelop?
• add travisci build meanwhile jenkinsosx doesnt work?

just ask, make a possible roadmap and ask community to help

[7sharp9]

Just being able to build visualfsharp from osx would be a great start.

I still need to run make / make install on osx on the fsharp/fsharp repo to test anything but at least I would know my PR actually builds :-)

[enricosada]

Just being able to build visualfsharp from osx would be a great start

👍 about that, with a simple travisci. @7sharp9 can you open an issue?

[dsyme]

I've been through the PRs and only 6 remain that are not Work In Progress (WIP) https://github.com/Microsoft/visualfsharp/pulls. I'll close this discussion thread since things are in fairly good state.

[panesofglass]

Thanks for your work on this, @dsyme.

[dsyme]

The vast majority of the work was done by @KevinRansom. Please take the time to give feedback to Microsoft management as requested in #877 :)

[panesofglass]

Thank you for your work here, @KevinRansom!