CoreFX contributor experience #22080
Comments
|
Here's what I think we should aim for as a new contributor experience (I am nothing if not ambitious):
There may be emergency medical assistance required in the build dept now as people are dying of laughter... |
|
I have seeded top post with a list of key VS scenarios from @FransBouma's list in https://github.com/dotnet/corefx/issues/4571#issuecomment-305128570. Anything else troublesome missing? The same scenarios on command line are already either documented or listed in Contributing docs
cc @haralabidis |
|
There needs to be a guide which helps the contributor determine which type of contribution he is making (e.g. bugfix or API addition) and depending on the type, lists exact and complete steps required for each type of contribution. This includes determining which files and projects to modify in coreclr and corefx, and every esoteric detail like knowing when and how to regenerate baselines and include them in the PR. Instructions on when and how to run tests targeting netfx and write tests targeting netfx (or other platforms). In other words, everything you'd do as a corefx team member when you write and test code for a PR needs to find its way into this document. And ideally this needs to be all in one document and easy to get to from CONTRIBUTING.md and README.md. |
|
@willdean I merged your list into top post. I made ReSharper/NCrunch as P2 - as P1 we need one way to do it, additional ways are nice-to-have, hence P2. |
|
@karelz OK - as long as that P1 test runner allows things like debugging into tests. I do understand that getting 3rd party stuff working is possibly much harder than in-house stuff. (Cue more doubled-up laughing from around the gun'n'clouds org chart, no doubt...) Edit - I see this was your number 5 item, sorry. |
|
While writing code we often need to debug into one particular test over and over. This should just work every time via ReSharper and Visual Studio's test window. During my last contributions, the xUnit runner was acting up and preventing me from running tests in the IDE at all. I had to compile Just a bit earlier, there was no way to even build coreclr and corefx with VS2017. This is well after it RTMed. There needs to be some kind of integration test guaranteeing that the latest RTM VS update can build, run and debug a single test and all tests. |
|
@jnm2 To run a single test, I found how to do that and documented it here: https://github.com/dotnet/corefx/wiki/Build-and-run-tests#running-or-debugging-a-single-test-in-visual-studio No idea if that always works in all cases, it did for a netstandard2.0 compliant test project :) |
I believe the docs / their future TODO list is already captured in Contributor docs -- see Small change and Expose API in CoreFX
Regenerating baselines should definitely be part of the how-to-steps, once they are filled ;)
Captured in Advanced changes. Adding netfx tests is not that super-common. I would like to prioritize the mainline scenarios first. But fair point it should be eventually documented as well. Note that we have even gitter chat room |
It's a good idea, I don't think such integration test belongs into CoreFX test suite though. Also automating VS is not fun. It is rather involved and heavy-weight test infra, but maybe we can leverage something from VS team ... added to the top-post
Good point, added to [5] in the master list. |
I think the priority should be getting all that together in one document (not a web of wiki links) which doesn't require people to scavenge from conversations in random PRs. I dislike the wiki because it does not evolve alongside changes to the build scripts. Ideally every time you touch the build scripts, you review the contribution guide and update the guide in the very same PR if it needs an update.
My point is not particular detail is the problem, but rather the fact that there seem to be so many of them that contributors are likely going to run into something no matter what they do. Each of my contributions hit a different set of undocumented issues. Ideally the documentation is comprehensive. Otherwise, it begins to feel like death by a thousand paper cuts. |
Read the intro first ;) -- wiki is temporary solution for easier collaboration, not a permanent solution. Once the docs are solid, we will flip them to normal docs in repo.
Agreed. Do you have any specific recommendations? |
|
Yes, well, here's to moving past the temporary. =D I still don't have a good idea what things like the baseline compat do. My recommendation would be to get on the people that know things and make them write them down. :-D |
|
My order of descending preference for any error
In the case of updating baselines, if you're talking about the API compat one, it should ideally update it itself. Less preferably the error should say |
Here's a suggestions - do a deal with the guy who writes NCrunch to supply the entire .NET Core team at MS with a copy. This will have numerous advantages:
|
To help them as they develop live unit testing! Oh wait.... :D |
I'm not sure the bit about Visual Studio is getting through :-) |
Oh - ha, fair point. I worked on Visual Studio for 15 years and it's great, but I admit I am using Sublime right now: to change it up if nothing else. I do believe most developers in the team use VS, but their tolerance for pain is high and/or they've learned to work around it hence this hasn't received the attention it deserves. (Pain in our systems/build not in VS itself per se) |
|
@danmosemsft This is exactly why VS is crap - because people like you (i.e. smart, influential, MS-insiders) have given up on it, and people like "The Scotts, Various" apparently only run keynoteware on completely stripped-down clean machines. Is it de facto MS policy that the tool that we fools all spend 1K+/year/seat on isn't actually any good for serious development? |
|
Gotta say I don't mind me a good bit of command line, but there's something to be said for automation. @willdean honestly, VS isn't crap and I see them run VS all the time... |
|
CoreFx has some unique requirements around running tests on specific frameworks. Whatever method is done that needs to be preserved. Today this happens through the configurations of the test projects, with .NET core being the default for netstandard libraries. I think configuration here is a bit limiting because ideally you want to run the netstandard tests against desktop, netcoreapp, uap, uap w/ netnative, etc. That's what'll actually happen in CI. This is discussed a bit here: https://github.com/dotnet/corefx/issues/20326. So add that to the wish-list: select the configuration on which to run a test. @karelz I think I'd expand your list to include some of the following e2e scenarios:
Things out of scope from VS, but definitely should have docs:
I agree that some high-level VS-automation type tests would be good to add to the repo. Perhaps as a separate leg in CI. This is definitely do-able using the DTE. |
|
@willdean I hear you. As I mentioned, the default in our team is VS or VS Code. We care about the .NET coding experience (general .NET, not our repo in particular) from any editor, so I think it's worth at least one of us using other tools. Specifically our sister team Roslyn, is used in a plugin for Sublime that I use. I'm not using Sublime because of any problem with VS. VS is my favorite app in the world and before I came to .NET I was one of the longest serving developers on Visual Studio. Let's focus on getting the VS experience better. Edit: incidentally the vast majority of Microsoft developers internally use Visual Studio, and many of the others use Visual Studio Code :) |
|
@jnm2 I've had an MSDN sub for about 20 years and also used every single version of the MS dev tools since C4.0 (that's after the moon landings but before the fall of the Berlin Wall) - you'll be hard pressed to change my opinion about VS merely by asserting a contrary one! But actually you're right, it isn't really crap (I wouldn't use it if it was), but it is dog slow and the historical failure to grasp the 64-bit nettle is now leading to a terrible train wreck of child processes as everything ambitious gets moved out into its own process. I don't want to get into endlessly moaning about VS here, but my feeling forever has been that if people working on big projects inside MS actually used it seriously, then it would have to be better. |
|
cc @vitek-karas who had troubles using VS recently. Note: @weshaggard pointed out that we have VS support in our backlog of engineering work, quite high up. We will have to find the right resourcing to make it happen. |
|
@weshaggard also pointed out offline that there are technical (and good) reasons for CoreFX repo not be able to work well in VS ever - e.g. our build has specific which interact badly with VS. Although there are some things we can improve (maybe test debugging?). It all boils down to building the platform itself from a tool that is very integrated with the platform. Kind of chicken and egg problem.
All projects in MS which can use VS, do use it. There are also large projects, like VS itself AFAIK. What is different on CoreCLR and CoreFX is that they are the platform itself being integrated tightly with the VS engines. Let's wait on details from @weshaggard before starting further discussion. Just wanted to give you all heads up. |
Essentially every developer working on VS uses VS, usually a recent internal build. Many/most people in Developer Division have a script that updates it overnight, in fact. Outside of VS it is the dominant dev environment. |
|
@danmosemsft Very encouraging! What a chilled-out and patient bunch they must all be :-) |
|
I love all the energy around making our tools better, it makes me very happy that so many folks are passionate about our project. First let me say there is a lot of room for improvement in the developer experience (in particular VS) when contributing to code in corefx which we should definitely call out in this issue to track and get improved. However as @karelz hinted at there are some reasons which make corefx libraries special that make interacting with the tools in the traditional way a little more difficult. Here are a few of those issues:
All that said we really do want to make working in VS and VSCode a first class experience for corefx contributors and so we should definitely push to make improvements but folks need to keep in mind the constraints we are working under in corefx.
This has come up numerous times but anytime we have attempted this nothing good happens because of the number of libraries and the complexity of the projects that we have in corefx. This is why we have been sticking with having one solution per library which contains all the relevant projects for that library. |
|
@weshaggard Thanks for this - it's probably about what I expected, and I can see that when you get to a low level there is a clear bootstrapping issue / circular dependency which is very difficult to resolve in a conventional VS-style build. However... There are (or could/should be) lots things in coreclr which really shouldn't be special at all - the leaf nodes of the dependency tree if you like. So like System.Drawing (which is partly what kicked-off this discussion) or System.Io.Ports. These are (or should be) ordinary class libraries, and it's not obvious to me that when MS builds a class library it should need dark magic but when the rest of us do it we just fire up VS and go "File -> New -> Class Library" and get on with it (that was a time-lapse description of the process). If I was to write an enhanced implementation of some high-level bit of CoreFx (not String or DateTime, obviously), I should be able to do that in VS without having to sacrifice a goat. I suppose in some ways I think that VS support is not really much of an end in itself (anybody likely to contribute to CoreFx is going to be capable of running stuff from the command line). Instead, VS support is a really good acid test of whether the development process is still anchored in the real world rather than in MS-land, where things are (understandably) different - you make your own gravity and the rest of us get that. "Can't be built/tested/debugged in VS" should be an exception which needs justification, not the default position, which is what it is at the moment. Anyway, none this from me is searing insight, so I'll leave it there - thanks for the info anyway. |
|
@willdean thanks for the candidate feedback we really do appreciate it and want this to be a conversation and not one-sided.
Completely agree there is a line where higher level libraries can and should be just normal libraries. We have had numerous discussions about moving these libraries to another repo higher level then corefx itself which wouldn't have to use special tools, we haven't yet funded the work to define and move such libraries but it is something we want to do eventually. In the mean time we are trying to stay as consistent as possible with all the libraries that live in corefx which for better or worse means they all get this treatment. |
|
@weshaggard Understood. So to get to something workable, I think there are a couple of different actions a developer who wants to contribute to CoreFX has to perform, in any given order. That they can't all be done in vs is a bit of a pain, but for now it is reality, however the question remains: How to do them at all ?
There are likely more. Everyone in the CoreFX team has to perform one or more of these actions to do their job. How do they do that? If it is required to edit 15 files and run commands on the command line, for now that's what it is, but what are these 15 files and what commands do they run? There's info on some of them, and some are easy to answer (editing code, e..g you can do that in any text editor), but others aren't and are unknown, have contradicting info. To contribute code to CoreFX, an external developer needs the same info as what a new member of the CoreFX team is told, and perhaps with a little bit more details as we aren't in the same office building as the people who work on it for a long time already :) I'd like to know specifics, how to do the actions a developer has to do on the corefx framework, in a list, like the list above. Multiple people can contribute things to that list, and someone can then compile a document from that so contributors can take that document and get started writing code. Makes sense? |
|
I can try and outline some of the answers: Editing code You can of course use any text editor you want and build from the command line but if you want to edit in VS and get intellisense and stuff you need to follow the steps at https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md#build and do at least one root level build.cmd first. Compiling a project Also covered at https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md#build. If you feel that we need more targeted instructions we should definitely work on improving the docs. Creating a new project The general approach most folks take is to find an already existing project that is similar and copy/paste the directory and update the files to be correctly named for your new library and gut the source files. For specifics about the different type of projects and configurations you can find them at https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/project-guidelines.md. Creating a new test project for a given specific platform (so not .netstandard, but e.g. .netcoreapp) The configuration system information can be found at https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/project-guidelines.md#aggregate-build-properties. Once you define the new configuration you can update the VS project/solution by running the update command https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/project-guidelines.md#updating-configurations. Creating a new test for a given specific platform in an existing test project which doesn't necessarily targets said specific platform While the docs don't specifically answer this exact question all the information to setup a new configuration is covered at https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/project-guidelines.md. Testing a single test There currently isn't any great way to do that today in VS, you can do it at the command line if you specify the right options. We should add that information to our developer guide https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md. Debugging a single test Your workaround is an interesting approach but I'm sure we can and should do a lot better with testing integration in VS. There is a lot of complexity and we have a lot of documentation on that overall system but we should work together to improve the documents to provide more tasked oriented information. What approach do you guys think works best? A bunch of overall documents or an FAQ style question and answer approach? |
|
@weshaggard I am not a great fan of documentation, either writing it or reading it, particularly where its primary function is to paper-over things which could be intuitive but aren't. An idea which occurs to me on the spot and hasn't had much thought - could there perhaps be a sort of 'template' (i.e. example, not some IDE-specific template magic) of a library + associated tests, which could function simultaneously as a starting point for a copy/rename process, as an example of a component of CoreFx, and as a repository for documentation? So, for example, we could have the Obviously the final build/packaging process would not include this library in any distribution, but it could in all other respects be a real part of CoreFx. Hey, it's Friday... |
|
@weshaggard, I don't see how any of those reasons make it harder to develop in VS.
While the listed problems are things to be overcome (and have been on cmd line) when developing for corefx, basically what I'm asking is why do these things make it harder to get things working in VS? I'll take a component of the issue to demonstrate what I'm asking. VS supports test discoverers being added to a project via nuget. This is passed parameters about the project and will return metadata about the tests in the project. There exists an xunit test discoverer package which works for .Net Core projects so there's proof by implementation that there's no technical barrier to having a test discoverer running on in VS which can discover tests which will execute on .Net Core. There's also no technical barrier to actually running xunit tests on .Net Core and having the results appear in VS, I've done it. Once build.cmd has run and bootstrapped everything, why can't we discover xunit tests in a corefx test project in VS? |
I believe the implication here is that some folks want to be able to do EVERYTHING from Visual Studio, without requiring any "manual" steps to be run beforehand.
Because nobody has written the NuGet package you are describing. I don't think anyone has said there is a technical reason we can't; it just hasn't been considered a priority. Actually, the team has historically created multiple runners which integrate with the Test Explorer, but those are no longer maintained or working, and were built for older engineering systems. It's not just that building the NuGet package itself is expensive, but also maintaining it and ensuring that it stays in sync with build system and product changes, is very time-consuming and fragile.
What @weshaggard was getting at is that Visual Studio struggles with the sheer number and variety of configurations that we support in corefx. There is no clean way to represent all of the configurations we have in a single Visual Studio solution file. Do we just list 50+ configurations in the drop-down menu? 100+? We're certainly open to suggestions about how this could be represented, but we've punted on the problem for now, because testing .NET Core libraries on Windows was the most obvious need for most VS users. |
|
I think it's clear there's a divergence in tooling between MSFT and external contributors. External contributors, myself included tend to use VS compared to the command line, as we've seen here. |
|
I should add that how things are right now in regards to UWP reminds me of the netstandard2.0 push in the first couple of quarters of 2016. It was unclear what was going in to netstandard2.0 as it was an evolving product and most effort was around the porting-to-core which by nature is hard for contributors. |
That's correct. I hope @weshaggard's answers explained the reasons behind - IMO they are good reasons, not just slacking off / being evil from our side.
That is correct. I started pushing .NET Native team to make at least daily build available to enlist community help. Everyone seems to be onboard, the last missing piece is to actually finish the tooling internally and have someone publish the builds and steps how to use it (we started discussing WHO is going to do it earlier this week). cc @danmosemsft @Petermarcu @joshfree ho are key decision makers in the UWP tooling for community.
Can you please tell me more? Did it block your .NET Core contribution? If yes, I want to know more ASAP. That should NOT be the case. We would be against ourselves.
If everything goes as I think it will, we should have public ability to run UWP tests in next few weeks. That said, even if the worst happens and we won't have ability to run UWP tests in the open, it should NOT prevent .NET Core 2.1 contributions in CoreFX - the business should be the same as it was so far. We will keep taking bug fixes, new API proposals, etc. There will be also investments (potentially with call for help from community) on larger libraries like System.DirectoryServices, System.Drawing, and a few more. We can keep discussing these topics here - it is a great discussion I wanted for a long time. I believe it will be also great topic for CoreFX contributor standup discussion in 2-3 weeks (sometimes online back-and-forth is more productive). I'll ping top contributors in CoreFX soon-ish (@hughbe you're on the list 😉). |
I understand your point, however at the moment it's often 0 configurations work. I think many people would already be happy if they can build their code they've written in VS just for 1 configuration, write tests for just that 1 configuration, so their code at least works, and then branch out to command line to make sure it works for all configurations. If 0 configurations work, you have to do everything on the command line. That in itself isn't a problem, however commandline work requires you know what commands and arguments and batch files you need to use to get things done. For an outsider precisely that is the core issue. |
I advocated that because I think it gives the easiest "new contributor experience" - i.e. clone the repo, build the project. Given that the pre-requisites / bootstrapping can currently be built from a single command line command, it's not clear to me that it's any show-stopping problem to have that invoked by VS as a pre-build step. We run all sorts of weird stuff here from VS as part of everyday life. You do generally need some care that you're not going to put 90 seconds of dependency-scanning into every F6 build, but as long as you can deal with that then I'm yet to be convinced that, fundamentally, there couldn't be a project within CoreFx called'Build Prerequisites' which every solution then included as a dependency of its other projects. I'm not saying it's no work though, I acknowledge that. |
I meant tests (not texts) in my previous message, but I think that was obvious. I'm referring to #21044 in this specific example. I introduced tests that broke the UWP test run. I intended to contribute a fix to either the tests or the product code (that the tests exposed), but couldn't. |
The only case where 0 configurations work is when an initial root level build.cmd hasn't been ran first. Assuming that has been down then the .NET Core configuration should work in VS and if not then we need to get that fixed.
I wish it was as simple as that but unfortunately it isn't. Part of the bootstrapping logic is to pull down and setup our tools which include lots of props/targets files so if that hasn't been done yet and you load the project in VS it will either not load or be in an invalidate state. So if we did that bootstrapping as a pre-step at the beginning of each project build it wouldn't help because VS has already loaded the projects and we would need a way to force VS to reload the projects in order to build them, which isn't possible (at least not that I'm aware of) while you are currently in the middle of building that project. |
That seems fair enough tbh. Does |
build-tests.cmd depends on build.cmd already have been run. It only builds and runs the tests. There is an option to build.cmd to do both the libraries and tests ( |
|
I believe we can close this issue in favor of https://github.com/dotnet/corefx/issues/20627 which covers the unit testing & debugging part. |
ghost
locked as resolved and limited conversation to collaborators
Forked from discussion in https://github.com/dotnet/corefx/issues/4571#issuecomment-305198456
In nutshell: @FransBouma ran into lots of hardship when trying to contribute to CoreFX repo - mainly around using VS. @jnm2 and @willdean raised similar concerns and additional points.
Let's have the discussion here.
When there are actionable work items what to fix, or separate larger sub-discussions, we may moved them into separate issues. This issue is forum for the overall problem space.
List of key VS scenarios:
Ideas:
The text was updated successfully, but these errors were encountered: