-
Notifications
You must be signed in to change notification settings - Fork 36
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
Added new --all-branches option #244
Conversation
…epo and exclude certain branches by name.
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.
Thanks for this useful addition! I've added 1 remark about the naming of the new arguments.
src/main/kotlin/nl/avisi/structurizr/site/generatr/GenerateSiteCommand.kt
Outdated
Show resolved
Hide resolved
@dirkgroot I've moved this to be a draft PR for the moment. I want to test to see what happens when if it hits a branch that does not build correctly or doesn't even have a workspace.dsl file and then do a little error trapping to skip those branches. I think this will provide a better experience. |
@dirkgroot I think I have a good solution for site generation using --all-branches where a branch may contain malformed DSL or even doesn't contain a DSL file at all. My first thought was to just drop a try/catch around the diagram and site generator but this would result in the bad branches still being listed in the branch switcher dropdown. To get around this I had to loop the branches twice, first to generate the diagrams and detect the errors and then a second loop to generate the actual sites after the bad branches names had been removed from the lists of found branches. |
src/main/kotlin/nl/avisi/structurizr/site/generatr/GenerateSiteCommand.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/nl/avisi/structurizr/site/generatr/GenerateSiteCommand.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/nl/avisi/structurizr/site/generatr/GenerateSiteCommand.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Dirk Groot <dirkgroot77@gmail.com>
Co-authored-by: Dirk Groot <dirkgroot77@gmail.com>
Co-authored-by: Dirk Groot <dirkgroot77@gmail.com>
…eCommand.kt Co-authored-by: Dirk Groot <dirkgroot77@gmail.com>
…eCommand.kt Co-authored-by: Dirk Groot <dirkgroot77@gmail.com>
…ultiBranchSupport # Conflicts: # src/main/kotlin/nl/avisi/structurizr/site/generatr/GenerateSiteCommand.kt
…t loop to detect workspaces that do not compile.
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.
Thanks for the changes! I've added 2 suggestions to make the code more idiomatic and concise.
src/main/kotlin/nl/avisi/structurizr/site/generatr/ClonedRepository.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/nl/avisi/structurizr/site/generatr/GenerateSiteCommand.kt
Outdated
Show resolved
Hide resolved
Thanks for the code review. Kotlin is not my primary language so always good to learn a few new tricks. |
…ot documented as a default and probably shouldn't default it to that as it is GitHub specific.
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 to me. Thanks for your contribution!
Two new options have been added to the Generate Site command
--all-branches
--exclude-branches
This will allow the site builder to generate a site for every branch that is found in a remote repository and exclude any that have been explicitly named.
Not sure how to create a test for this, there doesn't seem to be any test for the existing site builder processes.