-
Notifications
You must be signed in to change notification settings - Fork 680
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
Analyzers
-> Restarting VS when simply changing branches is not acceptable.
#3477
Comments
There are options. At a high level, I think the one that would be most natural for everyone, since they already use it without even realizing, is sticking them into a nuget package of their own and just reference the package. Whether that package is local or actually published on a nuget repo is unimportant, as far as visual studio is concerned. If this were a corporate team where everyone were guaranteed to have at least a similar baseline environment, I'd typically publish to a local shared nuget repo. Since that's not what this is, one option is to "publish" the package to a directory in the repo for that "shared" nuget repo. Then it'll just be pulled in as part of the normal restore step. Or the same thing can be done, but not nuget-flavored, and just stick the assemblies themselves in a central place to be referenced. The nuget way has the advantage of being more adaptable to different CPU targets, though. |
Oh. Forgot to mention the second option I meant to mention.. There's a visual studio extension to address this shortcoming that basically just hacks it by generating new names for the assemblies each time, so they can be loaded "dynamically." It's an absolute kludge for sure, but it does seem to be a viable workaround. |
As for the first option (nuget packages or other pre-deployment that takes the project out of the solution), we could even just put them in their own repo under the org, and either publish them to nuget.org or as releases right here on github to be pulled in via nuget or any other mechanism. Basically, as long as the analyzer project is not in the same solution, the pain goes away. I had actually originally written it in a separate solution, but was trying to avoid anything that involved deploying anything, since I couldn't do that to test it out. |
And, with how the newer ones are intended to be designed in a more target-agnostic fashion, I could even just publish them on nuget myself as not specifically bound to or dependent on Terminal.Gui. 🤷♂️ |
My inclination (without me publishing them separately, anyway) is to do it via a bit of a hybrid approach, doing all of the following:
This is a quickly-doable change to the process that will remove the pain and make an alternate/more formal means of distributing and referencing any custom analyzers a much less pressing need (but still something I'd suggest is a good idea before tagging TG as beta). |
@dodexahedron I just setup my new ARM-based Surface Laptop 7. Excited to get working on it. Only to find the ``The current processor architecture is: MSIL ... The module 'xxx'reqjuires the following architecture: Amd64` For now I removed the settings from the psd1 files that required AMD64. I'd appreciate it if you made it a priority to fix all this stuff. It really negatively impacts my productivity for very little benefit. |
I did that on purpose because I didn't intend to support it where VS can't run. You don't have to pre-build the analyzers to do a CLI build. However, I don't currently have any hard requirements in the modules on CPU architecture. I'd just have to make a platform check in the places where VS is launched to skip them or let the user eat the error themselves. 🤷♂️ |
As for addressing the initial post issue here, this issue is semi-soft-blocked by other things that need to happen first, including #3557, which is a building block of some necessary and also some opportunistic changes over here in preparation for splitting that off. I've considered various intermediate options, to provide a workaround quicker, but the work to do that I felt is better spent doing it right once, rather than making multiple breaking changes for the same purpose over here. 🤷♂️ |
Side note: Nice to have another architecture platform in a dev's hands, for testing, regardless. 🥳 |
Hm. I didn't realize there were ARM builds of VS 2022 available. In that case, I'll be sure to keep things platform-agnostic there, too. So the changes you made removing the architecture are cool. I'll take that line out of the module manifests altogether, in a future commit, since the default is empty anyway, so it doesn't have to be in there. I wouldn't bother setting it to MSIL (which is equivalent to "AnyCPU"), either, since there are already other things that would make that redundant. You know... like being a PowerShell module, for one. I'm not really sure why that option even exists but whatev. 🤷♂️ |
Every time I switch branches I need to exit VS, re-build them, and restart VS. This is not acceptable.
On my fast desktop this is painful. On my laptop it's excruciating. It takes many minutes just to change branches and I do that frequently.
@dodexahedron can you please find a solution to this that does not require such a painful workflow. When I agreed to merge it, I was under the impression that I'd only have to do the song-and-dance if I actually changed something that utilized them. But that's not the case, apparently.
Given the current value
Analyzers
provides is extremely limited I'm prone to just ask you to yank it out for now unless there's a way of them working that doesn't provide such a cruddy developer experience.The text was updated successfully, but these errors were encountered: