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

Add 'Diagramming Guidelines' #59

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Conversation

jonavellecuerdo
Copy link
Contributor

Why these changes are being introduced

Provide guidelines on current best practices for diagramming used by InfraEng and EngX.

How this addresses that need

Add "Diagramming Guidelines" to MIT Developer Documentation Site.

Side effects of this change

NONE

Copy link

@ghukill ghukill left a comment

Choose a reason for hiding this comment

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

Just a couple of small suggestions, but overall looks good to me! Thanks for writing this up.

misc/diagrams.md Outdated

## Who is the intended audience

Before starting your diagram, it’s important to think about your intended audience. Who are you creating the diagram for and who is it meant to help? Your audience will inform the level of detail your diagram requires. For instance, a low-level, “nuts-and-bolts” diagram may be more useful for InfraEng, while higher-level diagrams may be better suited for other stakeholders.
Copy link

Choose a reason for hiding this comment

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

Instead of,

may be more useful for InfraEng

what about something more generic like,

may be more useful for engineers

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed ☝️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The text has been updated.

misc/diagrams.md Outdated

## Why do we need diagrams

Diagrams are useful for communicating the data flow within an application. Diagrams can be particularly helpful for complex data-intensive applications (e.g., [geo-harvester](https://github.com/MITLibraries/geo-harvester/tree/main/docs)). That being said, there are some applications where the flow is simple and straightforward enough to not require a diagram. In these applications, thoughtfully written code can serve as documentation. Diagrams also increase accessibility of our data applications, by serving as another medium through which developers and stakeholders alike can understand the tools we create to support the work of MIT Libraries.
Copy link

Choose a reason for hiding this comment

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

I might propose omitting the specific example here, just so these docs don't get brittle? I think "data-intensive applications" is sufficient.

(e.g., geo-harvester)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also agree with this ☝️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reference has been removed!

Copy link
Member

@JPrevost JPrevost left a comment

Choose a reason for hiding this comment

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

Solid write up. None of my feedback is blocking merge (i.e. you can make changes or not depending on your preference).

I can see potential to add additional sections, such as "how to make your mermaid diagrams accessible" and "where do we use mermaid diagrams", but neither should hold up this up from my perspective.

EngX will take the lead on adding "how to make your mermaid diagrams accessible" as we've talked a lot about that already. The "where" part (mostly pointing out we use mermaid in GitHub and Confluence) can be added at any time.

misc/diagrams.md Outdated Show resolved Hide resolved
misc/diagrams.md Outdated Show resolved Hide resolved
Copy link
Contributor

@ehanson8 ehanson8 left a 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 as well!

misc/diagrams.md Outdated

## Who is the intended audience

Before starting your diagram, it’s important to think about your intended audience. Who are you creating the diagram for and who is it meant to help? Your audience will inform the level of detail your diagram requires. For instance, a low-level, “nuts-and-bolts” diagram may be more useful for InfraEng, while higher-level diagrams may be better suited for other stakeholders.
Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed ☝️

misc/diagrams.md Outdated

## Why do we need diagrams

Diagrams are useful for communicating the data flow within an application. Diagrams can be particularly helpful for complex data-intensive applications (e.g., [geo-harvester](https://github.com/MITLibraries/geo-harvester/tree/main/docs)). That being said, there are some applications where the flow is simple and straightforward enough to not require a diagram. In these applications, thoughtfully written code can serve as documentation. Diagrams also increase accessibility of our data applications, by serving as another medium through which developers and stakeholders alike can understand the tools we create to support the work of MIT Libraries.
Copy link
Contributor

Choose a reason for hiding this comment

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

Also agree with this ☝️

* Update language to be more generalizable
* Add section describing "where" diagrams are used
@jonavellecuerdo
Copy link
Contributor Author

Hi all! Please see the latest commit, addressing your comments. Thanks!

Copy link

@ghukill ghukill left a comment

Choose a reason for hiding this comment

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

👍

@jonavellecuerdo jonavellecuerdo merged commit 6d288ce into main Jan 24, 2024
2 checks passed
@jonavellecuerdo jonavellecuerdo deleted the dataeng-diagramming-guidelines branch January 24, 2024 18:52
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.

4 participants