-
Notifications
You must be signed in to change notification settings - Fork 386
Remove unwanted code from project templates #622
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
Conversation
Love it, except I don't get the point of the button. I'd prefer that to be removed too. It's throw away code. Don't add throw-away to a starter template. |
The check failures regarding Helix appear to be because I’m a contributor outside of Microsoft, and as such do not have permission to kick off Helix runs (whatever they are), but the build tried anyway. We should take a look at this sometime. |
I see the arguments for making the changes, but I'd want to know the reasons behind adding that code originally before debating whether to remove it. |
Agreed, feels like the changes here are purely subjective. At the very least, this should go through the proper workflow for public discourse. We've skipped that step here. |
dev/VSIX/ProjectTemplates/Desktop/CppWinRT/PackagedApp/BlankApp/MainWindow.h
Outdated
Show resolved
Hide resolved
f4f747d
to
f81d716
Compare
f81d716
to
53d850d
Compare
The C++ version of item templates and project templates seem to nest the source file as top-level if it doesn't have a XAML file rather than the IDL or header file. This is inconsistent with current templates included in the C++/WinRT VSIX and others built-in to Visual Studio. |
@MikeHillberg, can you review this PR or find someone to help drive it to closure? |
Any update on this? The current template is driving me bonkers 😀 |
Current templates confused me. Why I must clean-up xaml and code-behind before everything? I'm using MVVM+IoC patterns which means no code-behind is needed, except ViewModel property. StackPanel instead of Grid looks suspicious too. First thought was "no Grid support?". As SDK user I like the changes and suggest to merge the PR. |
Templates now seem to include Microsoft copyright and MIT License notice? Any reason why this was done. It makes sense for generated files to be copyrighted, but templates shouldn't be (obviously you'll add your own code after creating an instance of the template and that's not copyrighted by Microsoft!). |
Closing PRs that appear to have been abandoned. |
@bpulliam is this the process now? Ignore community PRs long enough for you to just call them abandoned? |
Bob's closure of this PR is part of an slow moving effort to clean up the repository. This PR skipped the proper workflow (at the time) for public discourse/changes, has minor and subjective changes in it, and has had no real engagement since 2021. Seems like this fits the bill for closure to me. If the author feels strongly about this change, I recommend opening an issue to discuss and get sign off before making template changes. |
@riverar is right, but I also should add more context.
I am happy to have the conversation about this, or any other issue we resolve or close, as the goal is to bring additional clarity on those issues. |
Apart from bug reports, you are mostly going to delete them anyway. It's only wasting time for you. I did open a pull request to remove them from the C++/WinRT (UWP) templates but it was not merged due to not having enough people to main the VSIX. Windows App SDK, however, is well maintained and this change should be merged. Maybe as an example, the stack panel and button can be included in MainPage.xaml created with the project template, but not in item templates for page or user control. |
@JaiganeshKumaran "Well maintained" [spits out coffee]. |
@riverar Sure is more well-maintained than C++/WinRT, which is done for now. |
History? None of the WPF, UWP or WinForms templates ship with anything in them. They are blank and ready to throw content on. It's been like this for many many years. But yes we should have an issue to discuss this in I guess. I was merely pointing out the messaging of having a PR discussed, then assigned to a Microsoft team member to follow it through, only to be closed years later after no one actually following through on it, and no explanation why it was closed besides "stale". It's a really bad messaging to send to community members who spent time trying to improve the SDK, and there's nothing the community could have done here to prevented it from going stale.
I love that the templates are optimized for bug reports, and not for productivity 😁 |
Inspired by #574.