Skip to content

Comments

Added a new Starcounter Template#4

Merged
Feyrious merged 21 commits intomasterfrom
boilerplate-template
Apr 26, 2017
Merged

Added a new Starcounter Template#4
Feyrious merged 21 commits intomasterfrom
boilerplate-template

Conversation

@Feyrious
Copy link
Contributor

@Feyrious Feyrious commented Mar 20, 2017

The template contains boilerplate code for a new Starcounter app with
the folder structure and basic handlers and viewmodels in place.

@Feyrious Feyrious requested a review from per-samuelsson March 20, 2017 15:50
{
partial class HelloStarcounterPage : Json
{
void Handle(Input.AddVisitorTrigger trigger)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename trigger to action

[Database]
public class GuestBook
{
public string FirstName { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to just use name

{
public string FirstName { get; set; }
public string LastName { get; set; }
public string Visited { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to DateVisited

new MainHandlers(),
new CommitHooks(),
new PartialHandlers(),
new OntologyHooks()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ontology hooks should be removed

};
});

this.Visitors = Db.SQL<GuestBook>("SELECT g FROM GuestBook g");
Copy link
Contributor

@ErlendLandro ErlendLandro Mar 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be updated manually. It should be changed to adding the new GuestBook with this.Visitors.Add

@per-samuelsson
Copy link
Contributor

@threezool

Thanks for contributing!

@warpech @miyconst

Can you guys take a look at this too, from the app dev side? You would know better if this is something we are missing and/or if there are already plans for something similar. @Mackiovello is already busy reviewing the actual code, and I can take on me to review the technical aspects (i.e. the actual project settings, etc).

@@ -0,0 +1,7 @@
namespace $safeprojectname$.Helpers
{
internal interface IHandler
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this interface, having an internal abstraction just for the sake of registering hooks and handlers. I'd prefer removing that and just statically list relevant classes in main, invoking Register on each.

<template is="dom-bind">
<div align="center">
<h2>Hello Starcounter</h2>
<h2>Starcounter Gust Book</h2>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*Guest not Gust

});

this.Visitors = Db.SQL<GuestBook>("SELECT g FROM GuestBook g");
this.Visitors.Add(new VisitorsElementJson
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't need to be in the transaction

@per-samuelsson
Copy link
Contributor

@threezool

You request to merge this into develop, meaning 2.4. Note that we currently have a non-compatible leap 2.3 -> 2.4, meaning projects should be configured differently for each target.

The project template you provide is compatible with 2.3, but not 2.4. So either this merge should go into v2.3, or you should adapt to 2.4, i.e. develop branch.

Details are here: https://github.com/Starcounter/Starcounter.VisualStudio/wiki/Version-compatibility#how-do-i-migrate-a-project-from-21-3--24-or-vice-versa

<VSTemplate Version="3.0.0" xmlns="http://schemas.microsoft.com/developer/vstemplate/2005" Type="Project">
<TemplateData>
<Name>Starcounter Boilerplate Application</Name>
<Description>A boilerplate project for creating a C# application that executes inside Starcounter</Description>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do better here? Something that better distinguish this one from the "minimal" Starcounter Application.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In VS most project templates use 'Blank' as a prefix for a bare minimum project. I'm thinking we could maybe use that on the already existing project to match the convention in VS.

Then we could maybe use the 'Standard' or 'Basic' prefix for this template and adapt the descriptions there after.

</ProjectSubType>
<SortOrder>1000</SortOrder>
<CreateNewFolder>true</CreateNewFolder>
<DefaultName>Starcounter Boilerplate Application</DefaultName>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve too?

page.Visitors.Data = Db.SQL<GuestBook>("SELECT g FROM GuestBook g");
var page = new HelloStarcounterPage
{
Visitors = {Data = Db.SQL<GuestBookEntry>("SELECT g FROM GuestBookEntry g")}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a very beautiful line of code. Consider breaking out the Visitors into a separate HTML view + view-model and use a blending point (starcounter-include + Self.GET) to demonstrate modularity within applications. Or write page.Visitors.Data for readability. Another possibility is to just set Data and then attach to visitors in OnData.

@per-samuelsson
Copy link
Contributor

Implements: Starcounter/Starcounter#4114

The let's invite @jwillmer and ask for his review if he can find the time.

@Feyrious Feyrious changed the base branch from develop to master March 21, 2017 10:44
@jwillmer
Copy link

How can I test it? Do you have a vsix file for me or is it already integrated in a nightly starcounter build?

@Feyrious
Copy link
Contributor Author

@jwillmer

  • Clone the branch
  • Locate the folder Starcounter.VS.CSBasicApplicationProjectTemplate
  • Copy the content in to a zip file
  • Locate the folder %UserName%\Documents\Visual Studio XXXX\Templates\ProjectTemplates
  • Copy the zip file in to this folder
  • Open the VS Command Prompt
  • Type in the command: devenv /installvstemplates (no need to close/restart VS)
  • The template is located in the C# node when creating a new project

@jwillmer
Copy link

Sounds really time consuming. Let me give you feedback as soon as you have worked out how to ship it to the end user.

@Feyrious
Copy link
Contributor Author

Feyrious commented Mar 21, 2017

@jwillmer

I made a .vsix file for the template and it should work, first time i had to make one. The template should now also be under the Starcounter node with this installer.

StarcounterExtendedTemplates.zip

@jwillmer
Copy link

image

I have (only) VisualStudio 2015 on this machine.

@Feyrious
Copy link
Contributor Author

Yah i just noticed that its limited to VS 2017 atm, looking in to making it compatible for VS 2015 also.

@jwillmer
Copy link

jwillmer commented Mar 21, 2017

Try to give the VSIX a good name already since it is used to auto update and don't forget to always increase the version number after changes so that all systems will recognize it as an update.

@Feyrious
Copy link
Contributor Author

There, figured out how to set the appropriate version for the vsix installer.

StarcounterExtendedTemplates.zip

@jwillmer
Copy link

VisualStudio 2015:
image

<entry>
    <record>613</record>
    <time>2017/03/21 16:24:35.691</time>
    <type>Error</type>
    <source>Editor or Editor Extension</source>
    <description>Microsoft.VisualStudio.ExtensibilityHosting.InvalidMEFCacheException: The Visual Studio component cache is out of date. Please restart Visual Studio.&#x000D;&#x000A;   at Microsoft.VisualStudio.ExtensibilityHosting.FaultCatchingAssemblyLoader.&lt;LoadAssembly&gt;b__15_0(AssemblyName assemblyNameParam)&#x000D;&#x000A;   at Microsoft.VisualStudio.ExtensibilityHosting.Utilities.SafeGetOrAdd[TKey,TValue](IDictionary`2 dictionary, TKey key, Object lockObj, Func`2 valueFactory)&#x000D;&#x000A;   at Microsoft.VisualStudio.ExtensibilityHosting.FaultCatchingAssemblyLoader.LoadAssembly(AssemblyName assemblyName)&#x000D;&#x000A;   at Microsoft.VisualStudio.Composition.Reflection.TypeRef.get_ResolvedType()&#x000D;&#x000A;   at Microsoft.VisualStudio.Composition.LazyMetadataWrapper.Enum32Substitution.get_ActualValue()&#x000D;&#x000A;   at Microsoft.VisualStudio.Composition.LazyMetadataWrapper.SubstituteValueIfRequired(Object value)&#x000D;&#x000A;   at Microsoft.VisualStudio.Composition.LazyMetadataWrapper.SubstituteValueIfRequired(String key, Object value)&#x000D;&#x000A;   at Microsoft.VisualStudio.Composition.LazyMetadataWrapper.TryGetValue(String key, Object&amp; value)&#x000D;&#x000A;   at _proxy_Microsoft.VisualStudio.Text.Utilities.IWpfTextViewMarginMetadata_7e5c586b-2c09-40d6-893a-63049b022746.get_GridUnitType()&#x000D;&#x000A;   at Microsoft.VisualStudio.Text.Utilities.ContainerMargin.AddMargin(IWpfTextViewMargin margin, Lazy`2 marginProvider, Boolean trackVisibility)&#x000D;&#x000A;   at Microsoft.VisualStudio.Text.Utilities.ContainerMargin.AddMargins(IList`1 providers, List`1 oldMargins)&#x000D;&#x000A;   at Microsoft.VisualStudio.Text.Utilities.ContainerMargin.Initialize()&#x000D;&#x000A;   at Microsoft.VisualStudio.Text.Editor.Implementation.RightControlMarginProvider.CreateMargin(IWpfTextViewHost wpfTextViewHost, IWpfTextViewMargin marginContainer)&#x000D;&#x000A;   at Microsoft.VisualStudio.Text.Utilities.ContainerMargin.&lt;AddMargins&gt;b__25_1(IWpfTextViewMarginProvider mp)&#x000D;&#x000A;   at Microsoft.VisualStudio.Text.Utilities.GuardedOperations.InstantiateExtension[TExtension,TMetadata,TExtensionInstance](Object errorSource, Lazy`2 provider, Func`2 getter)</description>
  </entry>

Second start of VisualStudio worked without this error.

@jwillmer
Copy link

jwillmer commented Mar 21, 2017

File Structure:
image

Comments:

  • "Helpers" folder is a bad design. Everything can be a "helper". I would not recommend to promote this in a template.
  • It is a nice idea to use the project name in Handle.Get but be aware that I could also use "My space Project" and that this breaks the handler. (There is a function to remove spaces)
  • If I don't know Starcounter I might try to open http://localhost:8080 but you can't expect that I would know which path I need to enter in order to see the app
  • Add a README that tells the user the next steps
  • Use comments - use a lot of them! Describe every thing.
  • Add commented out functions that the user might probably need if he starts modifying the project
  • Add reference links to Starcounter manual sections to each class
  • Use also [..] for bindings in HTML and describe the difference
  • PartialHandlers is empty, why? For you the structure might make sense but you need to add code to demonstrate for what the user should use this class.
  • Add the Simplified DLL. It is especially a problem in the beginning to think about the Copy Local: false flag
  • Include some HTML element (like redirect) to showcase how to do it

If I browse to http://localhost:8080/StarcounterApplication1 I get:

System.ArgumentOutOfRangeException: Das angegebene Argument liegt außerhalb des gültigen Wertebereichs.
Parametername: Can not find referenced Html file: "/StarcounterApplication1/viewmodels/MainPage.html"
   bei Starcounter.HtmlFromJsonProvider.ProvideFromFilePath[T](String filePath) in C:\TeamCity\BuildAgent\work\sc-2824\Level1\src\Starcounter\Middleware\HtmlFromJsonProvider.cs:Zeile 58.
   bei Starcounter.HtmlFromJsonProvider.Invoke(MimeProviderContext context, Action next) in C:\TeamCity\BuildAgent\work\sc-2824\Level1\src\Starcounter\Middleware\HtmlFromJsonProvider.cs:Zeile 53.
   bei Starcounter.Internal.JsonMimeConverter.Convert(Request request, IResource before, MimeType mimeType, MimeType& resultingMimeType) in C:\TeamCity\BuildAgent\work\sc-2824\Level1\src\Starcounter.Apps.JsonPatch\BuiltInRestHandlers\JsonMimeConverter.cs:Zeile 18.
   bei Starcounter.Json.AsMimeType(MimeType mimeType, MimeType& resultingMimeType, Request request) in C:\TeamCity\BuildAgent\work\sc-2824\Level1\src\Starcounter.XSON\Class Json\Json.IResource.cs:Zeile 43.
   bei Starcounter.Response.ConstructFromFields(Request req, Byte[] givenBuffer) in C:\TeamCity\BuildAgent\work\sc-2824\Level1\src\Starcounter.Internal\Http\Response.cs:Zeile 809.
   bei Starcounter.Request.SendResponse(Response resp, Byte[] serializationBuf) in C:\TeamCity\BuildAgent\work\sc-2824\Level1\src\Starcounter.Internal\Http\Request.cs:Zeile 1241.
   bei Starcounter.Internal.AppsBootstrapper.ProcessExternalRequest(Request req) in C:\TeamCity\BuildAgent\work\sc-2824\Level1\src\Starcounter.Apps.JsonPatch\AppsBootstrapper.cs:Zeile 811.
ParamName=Can not find referenced Html file: "/StarcounterApplication1/viewmodels/MainPage.html"
HResult=-2146233086

More information about this error may be available in the server error log.

@per-samuelsson
Copy link
Contributor

It is especially a problem in the beginning to think about the Copy Local: false flag

FWIW, this is changed in 2.4. There, we are switching to other private assemblies (i.e. Copy local: true), which is the default for standard projects.

@warpech
Copy link

warpech commented Apr 4, 2017

Ping @threezool. This was a good initiative. Where are we on this?

@Feyrious
Copy link
Contributor Author

Feyrious commented Apr 5, 2017

Have had some other things to do but it is not forgotten. Just need to find the time.

@warpech
Copy link

warpech commented Apr 18, 2017

Let's not have so much work laying on the floor. Maybe it is best to merge as it is and split @jwillmer's comments (most, if not all, of which are valid) into separate issues?

@Feyrious
Copy link
Contributor Author

Its ok with me, i can transfer over the points in to separate issues to be worked on. It would also be easier to find the time if they are split.

@rotnot
Copy link

rotnot commented Apr 26, 2017

This PR is was not updated in a while. Please consider speeding up the review, splitting the changes into smaller tasks or anything else that makes it easier to decide about merging before it starts to smell.

@Feyrious
Copy link
Contributor Author

Merging and will add the points from @jwillmer as issues instead.

@Feyrious Feyrious merged commit 8556eb7 into master Apr 26, 2017
@mmnosek mmnosek mentioned this pull request Feb 2, 2018
@ubbeK ubbeK deleted the boilerplate-template branch February 8, 2018 11:41
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