Skip to content
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

Open
wants to merge 7 commits into
base: SRMFunction
Choose a base branch
from

Conversation

AndreRodriguesSkyline
Copy link
Member

  • 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
@@ -11,6 +11,9 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Newtonsoft.Json">
<Version>13.0.2</Version>
</PackageReference>
<PackageReference Include="Skyline.DataMiner.Dev.Automation">
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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

working/templates/srmfunction/SrmFunction.sln Show resolved Hide resolved
working/templates/srmfunction/SrmFunction.sln Show resolved Hide resolved
working/templates/srmfunction/SrmFunction.sln Show resolved Hide resolved
working/templates/srmfunction/SrmFunction.sln Show resolved Hide resolved
@AndreRodriguesSkyline AndreRodriguesSkyline changed the title Update Function Template DCP206202: Update Function Template May 9, 2023
Copy link
Member

@slc-peter-vanpoucke slc-peter-vanpoucke left a 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?

<Name>$PROTOCOLNAME$</Name>
</Protocol>
<Function id="$FUNCTIONID$" name="$FUNCTIONNAME$" maxInstances="0" profile="$PROFILEDEFINITIONID$">
</Function>

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.

Copy link
Member Author

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...

@PedroDebevere
Copy link
Member

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?
  • Changed file names to ABOUT.md
  • updated .gitignore to include **/.vs instead of .vs
  • Kept skyline2 for now as the way we see it currently is that customers could create their own templates based on this repo and adjust it as they see fit (e.g. by removing license text, etc.)

PedroDebevere and others added 3 commits May 12, 2023 14:16
- Remove extra comma
- Fix guids to match solution file
- Specify the required fields
Copy link
Member

@slc-peter-vanpoucke slc-peter-vanpoucke left a 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.

Copy link
Member

@slc-peter-vanpoucke slc-peter-vanpoucke left a 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.

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.

3 participants