Conversation
…ata; first draft of README.md.
…rectly. Restructured the solution's folder structure. Added github workflow files for test and release.
…that the Test project can consume it.
mpkorstanje
left a comment
There was a problem hiding this comment.
Overall looks good. But for consistency please use npm to copy the resources (if possible) and write the tests such that they don't depend on specifics.
| [InlineData("attachments")] | ||
| [InlineData("empty")] | ||
| [InlineData("backgrounds")] | ||
| [InlineData("examples-tables")] |
There was a problem hiding this comment.
Please find a way to test that the resources are packaged without relying on specific resources. Otherwise we'd have to update each implementation when test data is changed.
There was a problem hiding this comment.
I was following the approach used by the other languages (both Ruby and Python confirm that the packaging was successful by testing for the presence of known existing samples).
Is there an alternate approach elsewhere that I should be following?
There was a problem hiding this comment.
I was following the approach used by the other languages (both Ruby and Python confirm that the packaging was successful by testing for the presence of known existing samples).
They both shouldn't be doing that either.
Ideally you check if you find any .feature and and .ndjson file.
There was a problem hiding this comment.
OK. I'll adjust and resubmit.
| </None> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <Content Include="..\..\devkit\samples\**\*"> |
There was a problem hiding this comment.
For consistency please add a npm run copy-to:dotnet to copy the resources into your project. For example Java uses:
- name: Copy the samples to java/../features
run: npm ci && npm run copy-to:java
working-directory: devkit
There was a problem hiding this comment.
The <Content Include= > directive is the native/idiomatic way of doing this in .net projects. If we did have a mechanism outside the .net project that copied those files from the devkit folders into the project's folders, I would still have a <Content Include= > directive (just the path would be inner rather than traversing upwards).
Requiring the use of npm to copy the assets into the project structure means that the .NET developer needs npm installed and running locally in order to build and test. And we would need much more complicated and customized MSBUILD script to support doing this during local builds.
What type of consistency are wanting here? Consistency in use of npm to perform copy operations? To make it explicit that the resources get copied into the local language implementations prior to invoking their build processes?
There was a problem hiding this comment.
I'm looking mostly for consistency in process and a clear interface between the devkit project and the other modules. Right now the devkit is the owner of its own project structure, if another language implementation starts reading from it that is no longer the case and it becomes much harder to change the devkit.
Requiring the use of npm to copy the assets into the project structure means that the .NET developer needs npm installed and running locally in order to build and test. And we would need much more complicated and customized MSBUILD script to support doing this during local builds.
To do anything meaningful with this project you already have to use both npm + another build tool.
But could you elaborate on this a bit more? Aside from (manually) running a npm command to copy the samples, how does this affect the build processs?
There was a problem hiding this comment.
This PR makes use of a .net build directive for nuget publication that copies folder/s into the built nuget package. It retains the sub-folder structure.
Any changes that the CCK project might make to that structure might break a consumer that presumes a particular structure, but, IMO, that's on them. I would not expect the CCK to customize its build process/output structure to retain consistency over time (ie, a fixed content 'contract').
Re: use of npm:
I am not aiming at providing a .net implementation of a faux cucumber executor. My goal is to provide a .net centric way of packaging the contents of the devkit samples such that other .net projects (ie, Reqnroll) can consume the samples in their test suite(s). This PR packages the devkit/samples as content inside a nuget package. Reqnroll's test suite can then reference that nuget package and get access to the samples for use in its test suite.
Our (Reqnroll's) current process for consuming the CCK is manual. I download the samples to my local workstation and then copy & commit them into Reqnroll's test suite source code tree. The process is manual, error prone, and no one else on the core team understands it well enough to replicate it should I get hit by a bus.
By wrapping the samples as a nuget package, the samples are no longer directly committed as content in the Reqnroll source tree, minor updates are automatic, and the dependency is explicit.
If the only segment of the CCK that I'm interested in here is the content of the devkit-samples, and .Net provides a built-in mechanism for referencing that content in its build process, why complicate things? A dotnet dev, such as myself, can directly build and test the .NET portion of this project on my workstation without the need for npm or other tooling. The larger CCK build process already invokes the 'dotnet build' tooling. I personally try to keep the tooling surface area on my workstation as minimal as possible (e.g, I don't have npm installed). Making it a prerequisite complicates my life and potentially narrows down the candidate list of other dot net devs that we can recruit to continue to maintain this.
There was a problem hiding this comment.
Unfortunately that works both ways. Someone maintaining the devkit would have to know about every language that copies samples from it and how to fix that. And it's even harder to find people who work on all these languages. 😄
But that said, I can see you point as well. The less knowledge needed, the better. I'll see if we can refactor the devkit to put the devkit/samples folder in the root of the project. Then we'll have a clear interface.
As an aside, how you you build the html-formatter module without node?
There was a problem hiding this comment.
For now you can leave this as is.
There was a problem hiding this comment.
But that said, I can see you point as well. The less knowledge needed, the better. I'll see if we can refactor the
devkitto put thedevkit/samplesfolder in the root of the project. Then we'll have a clear interface.
Thanks for that.
As an aside, how you you build the html-formatter module without node?
You know, I had forgotten about that. IIRC, in that case, I loaded up the repo in a container, ran the make build to have the assets built, and then proceeded with .net dev from there.
So, I will admit that when push comes to shove, I can and do use tooling from other platforms. I just resist it at first.
If you find the refactoring of the location of the samples to be too onerous, then I will adjust and use the npm copy technique to get them prepositioned for the dotnet build.
🤔 What's changed?
This PR adds a dotNet implementation that publishes a package to Nuget. When consumed, the package will make the devkit sample assets available to the consuming project in their assembly's output folder as './cck/Samples'.
⚡️ What's your motivation?
Parity with other language platforms.
🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.