plantuml / plantuml

Generate diagrams from textual description
https://plantuml.com
Other
9.73k stars 881 forks source link

Any chance the c4 library could be merged in? #542

Closed vlovich closed 2 years ago

vlovich commented 3 years ago

Looking at https://github.com/plantuml-stdlib/C4-PlantUML, I see it as a much nicer way to layout diagrams. Any chance this could be adopted into this project so it’s more readily available within the ecosystem (I’m not a maintainer of it - just an interested party).

Potherca commented 3 years ago

@arnaudroques The only thing I am not 100% certain about is the issue regarding https://github.com/plantuml/plantuml-stdlib/issues/32. I'm not sure if the fixes we have in the C4 repo are comparable to those in https://github.com/plantuml/plantuml-stdlib/commit/462939f198400c4d5d15b1c6fd8c90986f8be7f4

I'm rather busy at the moment, @adrianvlupu / @kirchsth Does either of you have time to take a look at this? Otherwise we might have to wait just a little while longer before merging.

On a side-note, updating C4 would also resolve https://github.com/plantuml/plantuml-stdlib/issues/31.

talfco commented 3 years ago

Would be cool to get the latest C4 library (May) update integrated and having all the nice features out-of-the-box available. That would easen the process to make the C4 in an easy way available to a larger dev community in the company I work.

arnaudroques commented 3 years ago

Would be cool to get the latest C4 library (May) update integrated and having all the nice features out-of-the-box available. That would easen the process to make the C4 in an easy way available to a larger dev community in the company I work.

@talfco Well, the easiest way would that you submit a pull request to https://github.com/plantuml/plantuml-stdlib/tree/master/C4 with those updates.

We try to delegate the management of the standard library to the community. Once modifications are pushed to https://github.com/plantuml/plantuml-stdlib the official jar file will be updated accordingly.

talfco commented 3 years ago

So you mean, getting tagged version v2.2.0 (the last one, released in C4_PlantUML) puml files copied over into the C4 directory and then request a pull? I did that on my local machine in a local branch but would require then the right for the "pull request" on the plantuml-stdlib. If that is in line with the maintainers of C4_PlantUML.

arnaudroques commented 3 years ago

So you mean, getting tagged version v2.2.0 (the last one, released in C4_PlantUML) puml files copied over into the C4 directory and then request a pull?

Yes, exactly. You will also have to edit some file headers. For example, changing https://github.com/plantuml-stdlib/C4-PlantUML/blob/master/C4_Component.puml#L2-L6 to simply

!include <C4/C4_Container>

This is because you should not do external connection from the standard library.

It would require then the right for the "pull request" on the plantuml-stdlib.

Anyone could require a pull request :-)

If that is in line with the maintainers of C4_PlantUML.

Yes, it is. I understand that this may be confusing, but https://github.com/plantuml-stdlib/C4-PlantUML is the Unofficial PlantUML Standard Library Repositories. As maintainers of the official PlantUML standard library, we do support this initiative as people find more comfortable to work on an unofficial repository. But at some point, the unofficial work has to be copied to the official repository. And we welcome this :-)

I hope this is more clear, do not hesitate to ask some more questions.

talfco commented 3 years ago

Ok thx, so adjusted the header in the puml as well, the only missing piece: I can't push my branch due to permissions. My understanding is that I require this right in order to request a pull ?

16:45:31.207: [plantuml-stdlib] git -c credential.helper= -c core.quotepath=false -c log.showSignature=false checkout -b c4-v2.2.0 HEAD -- Switched to a new branch 'c4-v2.2.0' M C4/C4.puml M C4/C4_Component.puml M C4/C4_Container.puml M C4/C4_Context.puml M C4/C4_Deployment.puml M C4/C4_Dynamic.puml M C4/INFO 16:46:26.444: [plantuml-stdlib] git -c credential.helper= -c core.quotepath=false -c log.showSignature=false add --ignore-errors -A -f -- C4/C4_Dynamic.puml C4/C4_Deployment.puml C4/C4_Component.puml C4/C4_Container.puml C4/C4.puml C4/C4_Context.puml C4/INFO 16:46:26.645: [plantuml-stdlib] git -c credential.helper= -c core.quotepath=false -c log.showSignature=false commit -F C:\Users\felix\AppData\Local\Temp\git-commit-msg-.txt -- [c4-v2.2.0 756c004] Adding C4 tagged files fomr v2.2.0 7 files changed, 478 insertions(+), 370 deletions(-) 16:47:15.164: [plantuml-stdlib] git -c credential.helper= -c core.quotepath=false -c log.showSignature=false push --progress --porcelain origin refs/heads/c4-v2.2.0:c4-v2.2.0 --set-upstream remote: Permission to plantuml/plantuml-stdlib.git denied to talfco. fatal: unable to access 'https://github.com/plantuml/plantuml-stdlib.git/': The requested URL returned error: 403

arnaudroques commented 3 years ago

fatal: unable to access 'https://github.com/plantuml/plantuml-stdlib.git/': The requested URL returned error: 403

You have to first clone https://github.com/plantuml/plantuml-stdlib.git to your own repository. Then you will be able to commit the change to this clone. And you will submit a PR from that commit.

Tell us if you need some more help.

Thanks!

talfco commented 3 years ago

Ok yes make sense :) did a fork and requested the pull.

Potherca commented 3 years ago

As https://github.com/plantuml/plantuml-stdlib/pull/36 has been merged, this can be closed.

We will publish a new official release (V1.2021.6) in the incoming days with this updated library. Originally posted by @arnaudroques in https://github.com/plantuml/plantuml-stdlib/issues/36#issuecomment-841048991

Potherca commented 2 years ago

With https://github.com/plantuml/plantuml-stdlib/pull/42, PlantUML C4 v2.3 has now been merged.

Potherca commented 2 years ago

@vlovich / @arnaudroques Just a friendly reminder that this issue has been resolved (as stated above) and can be closed.