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

[TASK] Use auto-generated hierarchy in class diagram #518

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

oliverklee
Copy link
Contributor

@oliverklee oliverklee commented Mar 5, 2024

Now the only part of the class diagram which we need to maintain manually are the associations between classes.

Copy link
Contributor

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a Composer script to run the tool, with the arguments you are using, including the tool as a dev dependency? Currently, the rest of us are somewhat in the dark as to how you are generating this automatically.

I'm guessing that at present you don't have a solution for automatically updating the readme between the "start of" and "end of" comments, though that could ultimately be achieved. Assuming not, something that outputs an .md (or whaterver) file to be inserted would suffice for now. If it outputs a fixed file (rather than to stdout), then that filename would probably need to be added to .gitignore.

This could be done as a separate PR first, using the command line that created the current content before this proposed change. Then updated in this PR to also generate the inheritance rules that are being moved to auto-generated with this PR.

Changes themselves seem fine, though I have a query about whether inheritence rules from in-built classes should be included. They weren't before (specifically: SourceException extends \Exception).

Commentable <|.. Rule: realization
SourceException <|-- OutputException: inheritance
UnexpectedTokenException <|-- UnexpectedEOFException: inheritance
Exception <|-- SourceException: inheritance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should inheritance from in-built classes be included?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, there is no option to configure this behavior. So if we want to automate this, I propose we stick to what the tool generates.

@JakeQZ JakeQZ changed the title [TASK] Use auto-generated inheritance/implementation in the class dia… [TASK] Use auto-generated hierarchy in class diagram Mar 6, 2024
@oliverklee
Copy link
Contributor Author

I have create #521 for the Composer script and would suggest moving this change here first to get it out of the way.

…gram

Now the only part of the class diagram which we need to maintain manually
are the associations between classes.
@oliverklee oliverklee force-pushed the docs/class-diagram-inheritence branch from cf13c18 to bd81cd1 Compare March 6, 2024 17:32
Copy link
Contributor

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moving this change here first to get it out of the way.

Presume you mean merging this PR first? I'm happy with that.

I note the latest push is just a reordering and removal of the relationship with the built-in class.

@JakeQZ JakeQZ merged commit ac8b039 into main Mar 7, 2024
17 checks passed
@JakeQZ JakeQZ deleted the docs/class-diagram-inheritence branch March 7, 2024 01:25
@oliverklee
Copy link
Contributor Author

I note the latest push is just a reordering and removal of the relationship with the built-in class.

Yes - I only rebased on top of the current main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants