izar
Introduce TMSequenceConfiguration class to add functionality to PlantUML sequence generation
Attempting to implement some of my ideas from https://github.com/izar/pytm/issues/142 . This should be fully backwards compatible and self contained.
I'm not entirely happy with passing config attributes to the new sequence_line methods.
Example PlantUML output generated from the tests/sequence_config.plantuml file:
DeepCode's analysis on #6d1f02 found:
- :information_source: 1 minor issue. :point_down:
Top issues
| Description | Example fixes |
|---|---|
| The call to next should be guarded with a try/except block Occurrences: | :wrench: Example fixes |
👉 View analysis in DeepCode’s Dashboard | Configure the bot
👉 The DeepCode service and API will be deprecated in August, 2021. Here is the information how to migrate. Thank you for using DeepCode 🙏 ❤️ !
If you are using our plugins, you might be interested in their successors: Snyk's JetBrains plugin and Snyk's VS Code plugin.
so, while i recognize the utility of the sequence diagram improvements, I am still struggling with seeing this as 1st class functionality. Would it make sense to group sequence diagram functionality/improvements behind a SequenceDiag (or some such) object that we could change easily if we were to move away from plantuml at some point?
Would it make sense to group sequence diagram functionality/improvements behind a SequenceDiag (or some such) object that we could change easily if we were to move away from plantuml at some point?
I like that idea since I remember it was very confusing to me what's the relationship between both diagram types when I started with pytm. But we can do this in a follow-up PR and still get this one merged.
I would much prefer we don't incur the technical debt if we can avoid it. What do you think, @mikejarrett ?
I would much prefer we don't incur the technical debt if we can avoid it. What do you think, @mikejarrett ?
I don't see this as technical debt. This PR adds configuration to an existing feature of drawing sequence diagrams and as I understood you, we just agreed to refactor it into another class. If we don't want to do this right now and don't want to remove sequence diagrams completely, there's no reason to hold this PR hostage or reject it.
@izar and @nineinchnick , apologies for lack of communication it has been a busy couple of weeks. I'll see if I can come up with a different implementation in the coming days.
No worries @mikejarrett , I moved this to v1.1.4 in a month. I think it is a good one, we just need to encapsulate it.
Hi @mikejarrett do you have time to try and encapsulate this functionality before we merge? Thanks!