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

Add FST-1008 #359

Merged
merged 7 commits into from
May 7, 2019
Merged

Add FST-1008 #359

merged 7 commits into from
May 7, 2019

Conversation

cartermp
Copy link
Member

@cartermp cartermp commented Apr 16, 2019

Here's the proposal to migrate FSharp.Compiler.Service to dotnet/fsharp and archive the current repository.

  • Add section on compatibility rules (hint: it'll be semantic versioning with no binary compat)
  • Consider Fable fork
  • Goals for development of FCS without Windows or VS dependencies

@cartermp
Copy link
Member Author

Soliciting feedback from:

@ninjarobot
Copy link
Member

I'm in favor of removing forks and eliminating merging. I assume all active contributors have signed the CLA so they can continue to contribute?

@Krzysztof-Cieslak
Copy link

Before getting to any feedback I'll add @ncave and @alfonsogarciacaro to this discussion as it impacts Fable fork.

@ncave
Copy link

ncave commented Apr 16, 2019

IMO the impact on Fable would be minimal.
Can't comment on other aspects of moving the repo, I'll defer to the maintainers of FCS.

@cartermp
Copy link
Member Author

@ncave I believe moving to a dotnet/fsharp repo would require a fork of the entire repo, which is more stuff to keep on top of. We could set up a bot that attemps to auto-merge whenever there are no conflicts, but the total code churn will be higher. So that could be a downside.

@ncave
Copy link

ncave commented Apr 16, 2019

@cartermp
You are right that the churn will be higher, but at least it provides access to the latest F# features. And yes, we can switch Fable support from a PR on dotnet/fsharp (which will probably never get merged anyway) to a PR on a fork of dotnet/fsharp that gets updated only on demand.

I don't see a problem with that, in fact I personally like the elimination of upstream/downstream for reasons stated above, as long as cross-platform FCS development and tests don't require installing too many dependencies (it's a bit hard to contribute to the compiler right now because of all the prerequisites).

Other people of course probably have other opinions, so that's that.

@cartermp
Copy link
Member Author

@ncave Thanks! I've added two sections - let me know if you have any objections.

@ncave
Copy link

ncave commented Apr 17, 2019

@cartermp Looks good, perhaps add "Thou shalt be able to run the FCS tests" to the build section, unless it's assumed. (Sorry, couldn't resist the pun, it was just hanging there :).

@enricosada
Copy link
Contributor

enricosada commented Apr 19, 2019

Looks good to me, awesome to move forward to a single repo 👍

@cartermp i'd like to add some notes on dev flow, to make shared maintenance easier also for outside MS, and maintain current features.

Each point is independent.

  1. maintain git tag for all stable releases, like now
  2. maintain the use of a RELEASE_NOTES.md like now, because is useful for consumers
  3. no nightly, just signed prereleases from master branch to a myget feed
  4. maintain a shared ownership to publish a stable, using git tag on master who generate a signed package.

git tags for stable

now all stable release are tagged, and that help a lot for regression etc.
we cannot use same naming convention (27.0.1) but something like FCS-27.0.1 or FCS-v27.0.1 can be ok so doesnt collide with others tags.

use a RELEASE_NOTES.md

the RELEASE_NOTES.md is really nice and help track changes at hight level, instead of read all the commits
Hopefully the maintainer should update that as needed, but every stable release should have it.
As a note, it's now used to generate the version number itself.

prerelease flow

For me it's ok that just Microsoft can build the signed package, it mean it was built by MS CI and signed.

I think, if possible, we dont need the F# extension flow with nightly (that's strange for a package).

I think we just need a prerelease package for each commit in master branch, so is automatic for every green build, and zero maintenance.

If a commit is merged in master, mean has passed the approval of maintainers.

The signed package (as prerelease) can be built by the MS owned CI every commit in master and pushed to a myget feed.

shared ownership for publish a stable

If possible, having two different way of publishing (unsigned and signed), mean packages are different.

I think is possible to allow to maintain the publishing from community and the requirement for signing like:

  • a build with a git tag like FCS-v27.0.0 trigger a CI job on MS
  • in that build job, the package is generated without prerelease AND signed
    • optionally the build can be set as failure if RELEASE_NOTES.md is not up to date with release date.
  • the package after that pushed on nuget.org, automatically or with manual approval

Using Azure Devops is possible to do all that, having a different job for tags, and a manual Release job to publish (using secrets in azure devops)

Like that you can add two release jobs with different auth (MS and FSF), and each one use a different nuget api key, so is known the publisher (MS or FSF) but the package is signed

That mean community (the person choosed by FSF, not everyone) can trigger a new signed build of the package, using MS infrastructure for signing.
that mean the package is really co-owned i think

the build job configurations are owned by MS, not in some yaml, so no changes are possible outside MS.

that mean only the allowed community maintainer can tag a commit and generate the build:

  • you can check git tag and signature in the build job (with git commit signing too if you want to make it complicated :D)
  • if the tag is not ok (no right prefix, no right author), the signed stable package is not generated at all

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

Successfully merging this pull request may close these issues.

6 participants