Skip to content

fix(challenges): add class to backend challenge wrapper, fix basic no…#13164

Merged
BerkeleyTrue merged 1 commit intofreeCodeCamp:stagingfrom
no-stack-dub-sack:fix/backend-challenge-wrapper
Feb 28, 2017
Merged

fix(challenges): add class to backend challenge wrapper, fix basic no…#13164
BerkeleyTrue merged 1 commit intofreeCodeCamp:stagingfrom
no-stack-dub-sack:fix/backend-challenge-wrapper

Conversation

@no-stack-dub-sack
Copy link
Copy Markdown
Member

@no-stack-dub-sack no-stack-dub-sack commented Feb 5, 2017

Pre-Submission Checklist

  • Your pull request targets the staging branch of freeCodeCamp.
  • Branch starts with either fix/, feature/, or translate/ (e.g. fix/signin-issue)
  • You have only one commit (if not, squash them into one commit).
  • All new and existing tests pass the command npm test. Use git commit --amend to amend any fixes.

Type of Change

  • Small bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would change existing functionality)
  • Add new translation (feature adding new translations)

Checklist:

Description

  • added the class .challenge-instructions to the <Row> component surrounding the function that renders challenge instructions in the new backend challenge component
  • changed all the code examples in Basic Node and Express section to <blockquote>s to exemplify the changes and for consistency with rest of challenge seed
  • this will also allow us to use <dfn> tags in this section which previously would not have formatted correctly

NOTE:

I think this may cause some merge conflicts in other PRs if merged before them. #13155 and #13162 should be merged first to avoid this I believe.

@BerkeleyTrue BerkeleyTrue added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Feb 5, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

@raisedadead
Copy link
Copy Markdown
Member

@no-stack-dub-sack blocking for a merge post the others?

@raisedadead raisedadead added the status: blocked In a transient & temporary hold. label Feb 8, 2017
@no-stack-dub-sack
Copy link
Copy Markdown
Member Author

@raisedadead Yeah, sorry, meant to add the label. Once the 2 referenced above are merged, we can remove this label and review.

@raisedadead raisedadead removed the status: blocked In a transient & temporary hold. label Feb 13, 2017
@raisedadead
Copy link
Copy Markdown
Member

@no-stack-dub-sack I guess we are good to go after addressing the review comments now?

@raisedadead raisedadead added the status: blocked In a transient & temporary hold. label Feb 15, 2017
@no-stack-dub-sack
Copy link
Copy Markdown
Member Author

no-stack-dub-sack commented Feb 16, 2017

@raisedadead Yeah, the two PRs holding it up have been merged and the comments from above have been addressed, but I don't think anyone's given it a full review yet

Sorry for the late reply, I didn't see that the last PR holding it up had been merged

@no-stack-dub-sack no-stack-dub-sack removed the status: blocked In a transient & temporary hold. label Feb 16, 2017

This comment was marked as off-topic.

@raisedadead
Copy link
Copy Markdown
Member

@no-stack-dub-sack these changes LGTM, except for some formatting overhaul. Would you be interested in doing so via this PR.

Or we could take up later, I'll leave the choice to you.

@no-stack-dub-sack
Copy link
Copy Markdown
Member Author

@raisedadead Where, in the entire backend section? Or just in the section that I've already modified. Because as you know, I've only done basic node and express here, and the entire backend curriculum has code examples that also need to be wrapped in blockquotes. So I can do it for this section, but I think maybe the other sections should have everything done at once

@no-stack-dub-sack no-stack-dub-sack force-pushed the fix/backend-challenge-wrapper branch from 1769c8e to 9ec760f Compare February 18, 2017 01:21
@no-stack-dub-sack
Copy link
Copy Markdown
Member Author

@raisedadead Ok, I've made some updates, I think I caught most of everything, but not sure if some things like POST and GET should be wrapped in code tags to have them stand apart even when they're not explicitly in the context of code. They are also mentioned in this section several times before they're actually introduced, so maybe this could use some re-working and DFN tags in the right places.

This also will now close #13168 which I opened a while ago.

@no-stack-dub-sack
Copy link
Copy Markdown
Member Author

@raisedadead Did you get a chance to check out the changes I made since we last checked in?

@raisedadead
Copy link
Copy Markdown
Member

@no-stack-dub-sack sorry have been travelling a bit last weekend. LGTM. Lets add more formatting in further PR's.

I am yet to pull down locally and merge it in. I am okay if anyone beats me to it.

@BerkeleyTrue BerkeleyTrue merged commit b9c54fb into freeCodeCamp:staging Feb 28, 2017
@BerkeleyTrue BerkeleyTrue removed the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Feb 28, 2017
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