-
Notifications
You must be signed in to change notification settings - Fork 12
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
DCP206202: Update Function Template #5
base: SRMFunction
Are you sure you want to change the base?
DCP206202: Update Function Template #5
Conversation
AndreRodriguesSkyline
commented
Apr 19, 2023
- Add Functions file with some pre-populated content
- Profile Definition JSON file will be generated when creating the solution
- Profile-load script now has some initial code
- Add Definitions/Instances/Parameters sub-folders to the Profile Data folder
- Add Functions file with some pre-populated content - Generated profile definition json - Profile-load script now has some initial code - Add Definitions/Instances/Parameters sub-folders to Profile Data folder
working/templates/srmfunction/.template_config/dotnetcli.host.json
Outdated
Show resolved
Hide resolved
@@ -11,6 +11,9 @@ | |||
</PropertyGroup> | |||
|
|||
<ItemGroup> | |||
<PackageReference Include="Newtonsoft.Json"> | |||
<Version>13.0.2</Version> | |||
</PackageReference> | |||
<PackageReference Include="Skyline.DataMiner.Dev.Automation"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the code does not compile, I suppose the SRM nuget will get added here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, currently the code doesn't compile.
I was waiting for the SRM nugget to add it here. If you prefer we can meanwhile add the DLL here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK to wait for SRM NuGet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Maybe the dev/QA can be continued when the nuget package is there.
Some small remarks:
- Is there a specific reason to rename to readme files to about.md?
Maybe use capitals for the file name because that's more common. - Not related (but annoying): the gitignore in root is missing out on .vs in de subfolders
- The user symbol now shows "skyline2" by default. That'll be confusing for users imo. Use "domain\username" instead?
working/templates/srmfunction/.template_config/dotnetcli.host.json
Outdated
Show resolved
Hide resolved
<Name>$PROTOCOLNAME$</Name> | ||
</Protocol> | ||
<Function id="$FUNCTIONID$" name="$FUNCTIONNAME$" maxInstances="0" profile="$PROFILEDEFINITIONID$"> | ||
</Function> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
S: Include some examples for parameters, interfaces, graphical tags. Can be commented out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function file only needs the Id and profile Id to allow the user to make use of the function editor in DIS.
The idea is to use DIS to extend the function, so no need to insert examples...
|
- Remove extra comma - Fix guids to match solution file - Specify the required fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. There's one thing I've missed earlier.
working/templates/srmfunction/Profile Data/Definitions/ProfileDefinitionName.json
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good. As discussed, reference to SRM nuget will be added in a new PR.