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

Contributing First Steps #104

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

kopelli
Copy link

@kopelli kopelli commented Sep 16, 2024

This PR may wind up being a work-in-progress for a bit to flesh out #100; or at least define other issues to tackle further.

Overall tried to make the project easy enough to clone it local and run it from Visual Studio. To accomplish this, the key changes:

  • Update the project order in the Solution file
  • Update the launchSettings to not be absolute file paths that may not match with where the dev cloned the repo
  • Update the launchSettings to point to an existing Sample structure
  • Create a shared EditorConfig setting for Visual Studio (or VSCode if the extension is installed) to adhere to when modifying the codebase.
    • There are a number of build warnings under the default analyzer settings. I changed the severity on most of those to either be Suggestions or Refactoring Only (Silent) to remove the warnings from the build output.
      NOTE: This is not yet complete

Outstanding work:
[ ] Clean up/convert the 'CLI Commands.txt' to meaningful launchSettings profiles...or tests...or something?
[ ] Guidance on how to regression test the project
[ ] Update the README 'Getting Started' section to point to the CONTRIBUTING once it is fleshed out futher.

@kopelli
Copy link
Author

kopelli commented Sep 16, 2024

@SlavaVedernikov This isn't quite ready yet, but wanted to start getting some feedback on it so far. The EditorConfig is just one of those practices I prefer to have in place for open & closed source projects to keep the team on the same page. I personally prefer all severity settings to either be Disabled or Error, but that takes a lot of conscious effort to rework code. Having this baseline it can at least become clear when work is done to resolve a particular setting across the codebase.

I'm also having some problems figuring out a potential testing flow. Unit tests for something like this does not seem as valuable as end-to-end/integration type tests that actually exercise the overall flow. Perhaps tests that mimic the CLI Project's Program.cs logic with controlled args being passed along. I'm guessing that's essentially what you do at the command line with the 'CLI Commands.txt' lines, right?

@SlavaVedernikov
Copy link
Owner

SlavaVedernikov commented Sep 16, 2024

@kopelli In terms of testing, you're right, it's a bit tricky :)

There are two key testing context:

  1. I don't expect any changes in generated AaC or Diagrams (i.e. .puml) when I'm working on some logic or process improvements
  2. I know that either AaC or .puml will change when I'm working on adding new AaC elements or new types of diagrams or changing content of existing diagrams etc.

In context 1

  • I am using the existing AaC/.puml as the baseline
  • I run draw-diagrams.bat files for all samples with set "redraw-all=FALSE" when I'm working on diagrams generation changes
  • I run execute-aac-strategy.bat when I'm working on AaC changes
  • I expect no changes in the AaC/.puml as the baseline

In context 2

  • I run draw-diagrams.bat files for all samples with set "redraw-all=FALSE" when I'm working on diagrams generation changes
  • I run execute-aac-strategy.bat when I'm working on AaC changes
  • I expect changes, so I validate them
  • I create a new AaC/Diagrams baseline
    • In case of Diagramming, I run draw-diagrams.bat files for all samples with set "redraw-all=TRUE" to regenerate all diagram formats.

I'm open to suggestions on this process improvements/changes etc.

Because Visual Studio stores the startup project in the user settings for a solution, when one is not set up yet -- such as after cloning the repo locally and running it for the first time -- it will default to the first project listed in the .sln file.
Since the CLI project is the end goal, switching up the order in the file to fit this behavior.

SlavaVedernikov#100
It makes sense that the default execution should be against something that already exists in the repository.
Tried to make this match the 'draw-diagrams.bat' file in the directory, the output doesn't quite match any more.

SlavaVedernikov#100
This helps bring in a consistent set of settings that apply to the codebase as a whole, and do not depend on the individual's IDE settings.
This also helps be explicit on what analyzer settings are enforced vs. what ones the project does not wish to adhere to.

SlavaVedernikov#100
This way it's easy to test a change against all examples
This way it's easy enough to execute all executions in a single go.
@kopelli kopelli marked this pull request as ready for review September 20, 2024 19:11
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.

2 participants