Skip to content

fix(dev): rename superBlock with v7 to certSlug#41738

Merged
ahmaxed merged 7 commits intofreeCodeCamp:mainfrom
ShaunSHamilton:fix/use-better-naming
Apr 23, 2021
Merged

fix(dev): rename superBlock with v7 to certSlug#41738
ahmaxed merged 7 commits intofreeCodeCamp:mainfrom
ShaunSHamilton:fix/use-better-naming

Conversation

@ShaunSHamilton
Copy link
Member

@ShaunSHamilton ShaunSHamilton commented Apr 3, 2021 •
edited
Loading

Checklist:

  • I have read freeCodeCamp's contribution guidelines.
  • My pull request has a descriptive title (not a vague title like Update index.md)
  • My pull request targets the main branch of freeCodeCamp.
  • All the files I changed are in the same world language, for example: only English changes, or only Chinese changes, etc.

Related to #41563

Renames the superBlock variable to certSlug in the instances where the value contained -v7, as this is solely used for certification routing, and must never be confused with the actual superBlock.

@github-actions github-actions bot added platform: api Server application that needs familiarity with Express, Fastify, MongoDB etc. platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. labels Apr 3, 2021
@ShaunSHamilton
Copy link
Member Author

ShaunSHamilton commented Apr 3, 2021

@ojeytonwilliams I have scattered some comments throughout with suggestions. The main suggestion being to decide on what to do with:

challengeMeta.superBlock === "Scientific Computing with Python"
allMarkdownRemark.nodes.frontmatter.superBlock === "Scientific Computing with Python"
superBlockDashedName === "scientific-computing-with-python"

My suggestion is:

  • Call anything un-dashed as superBlockTitle (see below)
  • Anything dashed as just plain old superBlock

Also, the client/src/pages/learn directory with the .md files: This did come up in our meeting, but what is the best course of action here:

  • All <superBlock>/index.md files have superBlock as the dashed name
  • All other <superBlock>/<block>/index.md files have superBlock as the what I call superBlockTitle (un-dashed name).

Does my suggestion sound reasonable?

@ShaunSHamilton ShaunSHamilton mentioned this pull request Apr 3, 2021
Closed
4 tasks
ojeytonwilliams
ojeytonwilliams reviewed Apr 5, 2021
View reviewed changes
@ojeytonwilliams
Copy link
Contributor

ojeytonwilliams commented Apr 5, 2021

  • Call anything un-dashed as superBlockTitle (see below)
  • Anything dashed as just plain old superBlock

Sounds reasonable. If we can be consistent across titles, blocks and superBlocks that would be great. It's a bit long, but thing and dashedThing is probably the way to go.

ojeytonwilliams
ojeytonwilliams reviewed Apr 5, 2021
View reviewed changes
@ShaunSHamilton ShaunSHamilton changed the title fix(dev): rename superBlock with v7 to slug fix(dev): rename superBlock with v7 to certSlug Apr 6, 2021
@ShaunSHamilton ShaunSHamilton marked this pull request as ready for review April 6, 2021 14:35
@ShaunSHamilton ShaunSHamilton requested a review from a team April 6, 2021 14:35
ahmaxed
ahmaxed previously approved these changes Apr 9, 2021
View reviewed changes
moT01
moT01 requested changes Apr 13, 2021
View reviewed changes
Copy link
Member

@moT01 moT01 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The link to a cert on the superBlockIntro pages is breaking things for me:

Apr-13-2021 13-10-35

Changing this line to use cert.certSlug seems to fix it.

@ShaunSHamilton
Copy link
Member Author

ShaunSHamilton commented Apr 13, 2021

Thanks, for catching that, @moT01 .

ahmaxed
ahmaxed reviewed Apr 21, 2021
View reviewed changes
@ShaunSHamilton ShaunSHamilton requested a review from moT01 April 21, 2021 13:40
@ahmaxed ahmaxed requested a review from ojeytonwilliams April 22, 2021 10:48
moT01
moT01 approved these changes Apr 22, 2021
View reviewed changes
Copy link
Member

@moT01 moT01 left a comment

Choose a reason for hiding this comment

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

Well, I looked over this pretty thoroughly last time, and again this time. I couldn't find any issues.

LGTM 🎉

@ahmaxed ahmaxed self-requested a review April 23, 2021 16:32
@ahmaxed ahmaxed merged commit d3f59e6 into freeCodeCamp:main Apr 23, 2021
@ShaunSHamilton ShaunSHamilton deleted the fix/use-better-naming branch April 23, 2021 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahmaxed ahmaxed ahmaxed approved these changes

@moT01 moT01 moT01 approved these changes

@ojeytonwilliams ojeytonwilliams Awaiting requested review from ojeytonwilliams

Assignees

No one assigned

Labels

platform: api Server application that needs familiarity with Express, Fastify, MongoDB etc. platform: learn UI side of the client application that needs familiarity with React, Gatsby etc.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ShaunSHamilton @ojeytonwilliams @ahmaxed @moT01