Skip to content

Conversation

@mtreacy002
Copy link
Member

Description

This PR to add mentorship backend system visualisation diagrams to wiki.

Fixes #530

Type of Change:

  • Documentation

How Has This Been Tested?

N/A

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials

@mtreacy002 mtreacy002 force-pushed the issue530-add-diagram-to-visualise-backend branch from 9245207 to efc4069 Compare May 4, 2020 10:45
@mtreacy002
Copy link
Member Author

@nandini45, I remember you said I could ask your help to review the diagrams. Will you still be able to help me with this? Thanks beforehand.
Btw, the file I put in this PR (to go in wiki) has view only access. The file link I placed in the issue#530 description is with edit access so you can comment/modify the file if need to 😉.

@nandini45
Copy link
Member

@mtreacy002 yeah i will check it. give my reviews too. hope it helps

@nandini45
Copy link
Member

@mtreacy002

  1. i had a question in the mentorship/backend-app diagram in database you didn't add the files of database that are db_utils and sqlalchemy_extension. although they are not connected to other files. still they are inter connected to each other. any reasons why?
  2. and i think app/api is enough instead of app/api-utils.
    3.and one more thing we can have one image per page to be true reading the 2nd image is totally impossible. i cant read out anything in it.
    what does the second image consist of the code ? i know it might seems irrelevant to ask but i cant figure out.

@mtreacy002
Copy link
Member Author

mtreacy002 commented May 4, 2020

  1. i had a question in the mentorship/backend-app diagram in database you didn't add the files of database that are db_utils and sqlalchemy_extension. although they are not connected to other files. still they are inter connected to each other. any reasons why?

Good catch, @nandini45. I might have missed them accidentally. I'll add them now 😉

  1. and i think app/api is enough instead of app/api-utils.

I was also wondering about that. I put -utils coz I thought app/api by itself can confuse people to think that those are the only files related to app/api, but it actually only things to do with utilities classes. What do you think?

3.and one more thing we can have one image per page to be true reading the 2nd image is totally impossible. i cant read out anything in it.

I agree, which is why at first I put the link to the file so people can zoom in and out for clarity. But you are right, probably I will add per section images underneath the second image for a better representation 😃 .

what does the second image consist of the code ? i know it might seems irrelevant to ask but i cant figure out.

The second image is similar to the class diagram where the top part somewhat shows attributes and the bottom part shows methods of the class. I emphasise here on similar to because it's not quite an exact technique to describe a class diagram 😂

@mtreacy002
Copy link
Member Author

mtreacy002 commented May 5, 2020

Fix: System integration - with db_utils and sqlalchemy_extension.

Screen Shot 2020-05-05 at 4 19 05 pm

Closer look to classes
Data persistence layer:
Screen Shot 2020-05-05 at 9 44 12 am

Data access layer:
Screen Shot 2020-05-05 at 12 26 28 pm

Namespaces:
Screen Shot 2020-05-05 at 2 25 44 pm

app/api - general
Screen Shot 2020-05-05 at 2 26 49 pm

app/schedulers & utils

Screen Shot 2020-05-05 at 2 28 02 pm

@mtreacy002 mtreacy002 force-pushed the issue530-add-diagram-to-visualise-backend branch from efc4069 to 65a50c8 Compare May 5, 2020 04:53
@mtreacy002
Copy link
Member Author

Service layer classes (app/api/resources):

user.py
Screen Shot 2020-05-05 at 2 42 54 pm
Screen Shot 2020-05-05 at 2 43 33 pm
Screen Shot 2020-05-05 at 2 43 54 pm

admin.py
Screen Shot 2020-05-05 at 2 42 22 pm

common.py
Screen Shot 2020-05-05 at 2 41 50 pm

mentorship_relation.py

Screen Shot 2020-05-05 at 2 30 11 pm

Screen Shot 2020-05-05 at 2 39 54 pm

Screen Shot 2020-05-05 at 2 40 46 pm

Screen Shot 2020-05-05 at 2 41 17 pm

@mtreacy002
Copy link
Member Author

Hi @nandini45, I separated the classes by sections now. They look much clearer now (no zoom needed). It's just that I feel putting them in one page a bit too much. Maybe I should create different docs for different sections? Also, I didn't (haven't) put the testing classes there yet.
But if they are placed in different docs (pages in wiki), then it'll beat the purpose of having a glance at them as a whole 😂 😂 (no different to going inside class one at a time in our IDE). Maybe we just skip the 2nd image altogether and just have the one image (1st image) as an overview of system integration. What do you think?

@mtreacy002 mtreacy002 force-pushed the issue530-add-diagram-to-visualise-backend branch from 65a50c8 to f7b4ca0 Compare May 5, 2020 06:28
@mtreacy002 mtreacy002 force-pushed the issue530-add-diagram-to-visualise-backend branch from f7b4ca0 to 2703140 Compare May 5, 2020 06:40
@nandini45
Copy link
Member

@mtreacy002 yeah i think so too. because i think users need not know so much details. they can just fork clone and check that much details in depth. the main image that is the first one seems good and provides a lot description about the py files used in backend.

@mtreacy002
Copy link
Member Author

mtreacy002 commented May 5, 2020

Agree... So, should we keep it just with the first image and take out the class diagrams from the doc? Or keep it in there but on the same page not on different pages? I'm incline to just use the first image for overview and don't worry about using class diagrams for details 😂. @isabel, can we also ask your opinion here?

@nandini45
Copy link
Member

yeah i wanted to say. ask isabel for further more opinion.
for me i will say the 1st image provides a general overview which a user needs to know.

This diagram will give you a quick overview of what the backend system looks like.

## What does the integration look like?
![app_w_relation](https://user-images.githubusercontent.com/29667122/81039468-71bf2600-8eec-11ea-8a9f-149a9dda0a0f.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

These images can be stored on the repository itself, we can never be sure what are the longevity of these URLs that are taken from inline comments.

Copy link
Member Author

@mtreacy002 mtreacy002 May 5, 2020

Choose a reason for hiding this comment

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

Thanks, for your suggestion, @SanketDG. Storing the images in the repository is no question the most ideal way. The thing is for normal contributors like us who're not maintainers of the repository, we don't have access and can't upload files as we like. So, @isabelcosta and I viewed this method (uploading to PR comment and refer it in doc) as the next best thing as alternative. Hope this makes sense 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

@mtreacy002 Apologies, I do not seem to understand. You can always put the images in the working directory of the git repository and then just git push them so that they appear here, and then they can be referenced from the README files

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, @SanketDG if I confused you. @isabelcosta and I discussed this matter (the best way to put image on repository/.md files) here. Instead of uploading the files as binary files we decided the better way is to use reference link from images uploaded to comments like these ones here. Please let me know if this clear up your doubt. Or perhaps you have a better suggestion, we'd like to hear about it. 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, gotcha! I can see how this is beneficial (lesser repository size, smaller clone times) but Git can handle binary files pretty well but it also has a tradeoff, (offline READMEs not a possibility) But yeah, if the image sizes are a problem, then it's better not to add them to the repository.

Copy link
Contributor

@SanketDG SanketDG left a comment

Choose a reason for hiding this comment

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

I am not sure if this is an effective way to communicate the architecture of the applcation to beginners. It's way too noisy and even though it might seem useful to first at beginners, it could intimidate users with information overload that they may not even need to contribute. I could be in some sort of bias here to not accept some new way of doing things, but there are already proved ways to document your architecture using high level component sketches, walkthroughs or just simple writeups.

This looks really great and I can see how this simplifies understanding, but it shouldn't be the front facing resource for beginners. A beginner resource should just explain the several components that make up the backend from a top level view, and it should ideally (can be debated) in written form.


Disclaimer: I am very new to this community so please take my advice with a grain of salt.

@nandini45
Copy link
Member

@SanketDG i agree that is way lot information for a beginner or any user i suppose. even maya would agree. we both were thinking to minimal it to only the main backend page. we are waiting Isabel opinion on it too.

@mtreacy002
Copy link
Member Author

mtreacy002 commented May 5, 2020

@SanketDG, yes, as @nandini45 have said. If you looked at our previous comments you'll see that we're doubting placing the in depth class like diagrams which actually can be explored by the beginners themselves on their own IDE on the repo they've cloned. Thank you for adding your opinion here, anyway, we really appreciate it. We'll have to wait what @isabelcosta think of this (placing just 1st image as overview and skip the class diagrams). 😉

PS: I'm also a beginner and still learning, so, we're on the same boat 👍

@nandini45 nandini45 added the Category: Documentation/Training Improvements or additions to documentation. label Jun 3, 2020
@devkapilbansal devkapilbansal added the Status: Needs Review PR needs an additional review or a maintainer's review. label May 11, 2021
@devkapilbansal
Copy link
Member

devkapilbansal commented May 11, 2021

Why this PR is not reviewed till now? I added Needs Review here, however I am not sure if it is On Hold or not

@devkapilbansal devkapilbansal added Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Jul 25, 2021
@devkapilbansal
Copy link
Member

It was decided with @anitab-org/mentorship-maintainers that this PR will be closed in near future and a link will be added to the documentation for anyone who wants to look into this.
As this visualization is not that beginner friendly

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

Labels

Category: Documentation/Training Improvements or additions to documentation. Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: Add diagrams to Wiki to help visualise Backend system

4 participants