-
Notifications
You must be signed in to change notification settings - Fork 50
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
Progress #1
Comments
Thanks, I was going to suggest the same. Apologies for reworking the project structure - I've spend the past couple of days trying to make it build on the CI server. I thought there were some namespace issues with the At the moment I have lots of questions and design decisions to figure out. I'll raise them all up on here as issues - I'd welcome your thoughts. Also any help with building out the solution would be much appreciated. I'll have a look at the code in your fork later today. |
I think you're on the right lines on your fork, although it looks like you've hit one of the problems I did - which is the solution doesn't build. Once I updated all the nuget packages that fixed it. Try pulling the latest from this repro in to see if that makes it build. I finished my brain dump earlier, and it looks like you've already made some progress on issues #4, #5, #6, and #7. Let's move the conversation onto each of the issues. If you're able to get some of the issues working with tests and are happy with the code, I'll merge them in. In the meantime feel free to open pull requests and we can discuss the code as you build it out. |
I haven't had issues compiling my fork but could not compile yours, perhaps there is something in the kvm version we're running? I've had issues with VS CTP and the kvm caching compilation though preventing building even after the issues were fixed, restarting these usually resolved things. I'll have a look at creating some tests tomorrow but I'm new to git pull requests, never done one so I apologise in advance if I make a mess of it! |
It seems that upgrading the nuget packages to
Don't worry. Thanks for the first PR - I've had a quick look on GH and it looks good. I'll try to merge in in tomorrow. I typically use GitHub Flow, so let's use that on this project too. We need to get your |
Sounds good, I thought I would keep PR's small until I understand the workflow better and can get unit testing up and running to test some of the stuff I wrote. |
Pulling build changes from master
Ok, so I forked your repro and ran the following git commands to get it into a state where it's synced up with my repro. I'm by no means a git expert, so please make a backup of your local repro before trying this. You can see the result here. My objectives were:
Here are the steps: Create a new branch called
Push your new
Delete your
Add the offical R4MVC repro as a remote. We will used this later to pull changes into your fork:
Fetch the latest changes from the
Make a new
Overwrite your
I then rebased your
There were quite a few conflicts to resolve, but most of them were with the Finally I pushed the updated
Once this is done you should be able to pull the latest changes in from my repro into you
If you work in topic branches and raise pull requests when work is ready to integrate the GitHub flow should work. The trick is to keep your I will also work in topic branches in the main repro so you can keep track of what I'm working on. Feel free to open pull requests early for code review or to discuss options - I know I will be! Once a pull request is open you can continue pushing to the branch to update it. More info on pull requests here. |
thank you for taking the time for this. I followed the steps with one or two bumps but it really helped |
Just realised that I was just spamming the "New maintainers wanted" issue with project discussion, so thought to add an update here, unless there's a better place for this. So far I've managed to get MsBuild compilation working for AspNetCore projects, which means I can leverage a lot of the @wwwlicious's code and build on top of it. I've still been experimenting in artiomchi/T4MVCCoreLite, and I've finally got a working proof of concept. The POC is based around the dotnet cli / powershell helper that will regenerate the classes. I've also got a class library that has the base IActionResult interface and a tag helper to render links, which is referenced from the Host MVC project. The POC can now successfully generate a link using the following syntax: <a mvc-action="MVC.Home.Index()">Home</a> With this stage achieved, I'm planning to start applying this to the R4MVC codebase. I'm considering either waiting for #43 to be merged, or committing on top of it (which will require project renaming (R4MVC -> R4MVC.Tools, and creating a new R4MVC project for the IActionResult interface etc) |
@artiomchi thanks. Your PR #43 is now merged and we have a green build again. Regarding next steps, if you can work in a feature branch in your fork and open PRs from there that will work best (see this guide to the GitHub flow). You may need to delete your local Getting the dotnet cli wired up next sounds good. Regarding the tag helpers, do we need to introduce a new |
GitHub Flow sounds like a simplified, less strict version of Git Flow which I'm usually using (except for this pull request, actually =/), so that sounds good. Regarding the tag helpers, I'm not sure how much experience you have with ASP.NET Core so I'll have to make some assumptions here for the sake of the readers that haven't written any before. While ASP.NET Core has support for the old/classic html helpers from previous versions of MVC, it also introduced tag helpers, that can do the same thing, but in a more html like fashion. For example, both links below will generate @Html.ActionLink("link text", "Action", "Controller")
<a asp-action="Action" asp-controller="Controller">link text</a> The benefit of the latter is that you have full control over the output html, and you can add any new attributes that you want to the anchor. Again, both examples will generate the same output @Html.ActionLink("link text", "Action", "Controller", new { id = 12 }, new { @class="button-red" data_bind = "text: titleText })
<a asp-action="Action" asp-controller="Controller" asp-route-id="12" class="button-red" data-bind="text: titleText">link text</a> For the ASP.NET Core version of T4MVC I've currently added a mvc-action TagHelper that leverages that flexibility and the power of T4MVC. I used The example above will now look like this: <a mvc-action="MVC.Controller.Action(12)" class="button-red" data-bind="text: titleText">link text</a> Of course, classic Html helpers will be supported as well to provide backwards compatibility for people that haven't started using the tag helpers. My point in all of this is that the new library will have the same functionality as the classic T4MVC, so users will be able to use The generated action overrides are close to identical to their counterparts in T4MVC (e.g. here) and you can see the TagHelper code here to see what I mean |
P.S. Any preference for the indent character? I usually use the VS default on most of my projects (space), but I've seen some of the classes in the R4MVC source use the tab character. Ideally I'd like to make sure that it's consistent across the whole solution, but I'll defer to your choice since it's your project |
Yes, not a lot. I'm aware of them, but I don't have any production projects using MVC Core yet.
That sounds like a good approach.
Great. I missed that point before.
I don't have a preference. If you prefer spaces, then lets go with that. |
So far the progress is going pretty good. I've got a working POC with most of the core functionality in place and working for basic scenarios. Things that are working so far:
While there's still a lot of work to do (the code needs a lot of refactoring to make it more readable, missing functionality, handle more advanced scenarios, support areas, support view components, etc), it's a first working build in some time, which is pretty exciting. Once I add area support, I'll be dogfooding the project in my own production environment, but additional feedback and testing would be appreciated. The binaries are being auto deployed to a MyGet feed (didn't want it to be on NuGet yet). You'd need to install both
dotnet cli support will also be added later. I started with the Powershell variant since it was trivial to implement P.S. Visual Studio doesn't properly display IntelliSense and highlighting for tag helpers at the moment, so you might want to install this package to enable it: Razor Language Services (see more) |
One other important note. While the This is currently a limitation of the MSBuild process which is not cross platform at the time (v15.2). There have been talks about making it cross platform, which might happen in v15.3, but that's up in the air right now. Until then, the tools package is targeting .NET 4.6.2 |
@artiomchi thanks, it sounds like you are making good progress. Once you have the areas support working, do you think we have enough for a public alpha? Setting up a CI feed from this project has been on my todo list too, so I'll get that sorted too. |
Absolutely! I've got two last things I want to do before going to a public alpha - Html helpers (to go alongside the tag helpers) and View locators. Both are quite trivial, actually. The first is going to be mostly a port of the same extenstion class from the main library, with slight changes due to the framework classes/assemblies. And the second will initially follow the default folder conventions. Given some free time, I'll have a pull request within the next couple of days. The generated code is simple and should be stable (it's very similar to the code generated by T4MVC). I'm not too happy with the tool code - I mostly just started the refactoring, so there's a lot of work to be done still to make it clean, and let's not forget code testing. But that's all on the tooling side. |
@artiomchi @wwwlicious ok if I close this now in favor of #50? |
@kevinkuszyk fine by me 👍 |
@kevinkuszyk fine by me as well! |
Hi Kevin,
Thought we should move our discussion here since this is what we are discussing.
I've gotten a bit further on today with generating the controller partial classes with support for inheritance and basic generics. Your changes made for testing look good but I'm not sure how to pull them into my fork as our approaches are slightly different.
Take a look if you want and see if there is anything in there that is helpful, happy to help if I can, roslyn is certainly "an adventure" :)
The text was updated successfully, but these errors were encountered: