Skip to content
This repository was archived by the owner on Sep 25, 2025. It is now read-only.

Update diagram - #129

Merged
allella merged 2 commits into
freeCodeCamp:masterfrom
matjack1:update-diagram
Oct 22, 2019
Merged

Update diagram#129
allella merged 2 commits into
freeCodeCamp:masterfrom
matjack1:update-diagram

Conversation

@matjack1

Copy link
Copy Markdown
Contributor
  • I have read Chapter's contributing guidelines.
  • My pull request has a descriptive title (not a vague title like Update README.md).
  • My pull request targets the master branch of Chapter.

Following issue #54 and PR #70 I thought opening a new one since the previous one wasn't getting feedback.

This PR is following the discussions we had in the issue and PR:

  • update of the link in README to point directly to the image, so that we don't need to update the file itself anymore
  • update the diagram using DBeaver
  • I've fixed the SQL schema so that it can be imported without changes

So that you can simply import the SQL file without having missing
definitions
@matjack1matjack1 mentioned this pull request Oct 22, 2019
@allella

Copy link
Copy Markdown
Contributor

Nice.

Only thing I noticed is since the link in the README is absolute to the upstream Chapter repo it will also show that image in anybody's fork. That's probably not ideal if we can make the link relative to the repo and branch. That may be more complex than it sounds, so I'm not going to hold this up on that question.

Squashing and merging since the existing diagram is way out of date.

@allella
allella merged commit e67006f into freeCodeCamp:masterOct 22, 2019
@allella

Copy link
Copy Markdown
Contributor

Thanks @matjack1.

One minor thing for future PRs, I believe the CONTRIBUTING.MD for the project would like branches to use prefixes of

fix/, feat/, docs/

@matjack1

Copy link
Copy Markdown
ContributorAuthor

thank you @allella I'll do that next time! :)

And yes, I wasn't sure how to make the diagram relative, we can try if it's necessary on future changes.

@matjack1
matjack1 deleted the update-diagram branch October 22, 2019 21:12
@allella

Copy link
Copy Markdown
Contributor

I think we should try to make it relative because it already tripped me up. I was looking at your fork's README and seeing the out of date schema, since README was pointing to the main repo's image before that version of the file existed there.

@nik-john

Copy link
Copy Markdown
Contributor

@all-contributors please add @matjack1 for doc

@allcontributors

Copy link
Copy Markdown
Contributor

@nik-john

I've put up a pull request to add @matjack1! 🎉

@freeCodeCampfreeCodeCamp deleted a comment from allcontributorsBotNov 16, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@matjack1@allella@nik-john