Skip to content

revise copy for intro array challenge#12817

Merged
dhcodes merged 1 commit intofreeCodeCamp:stagingfrom
no-stack-dub-sack:fix/array-copy-update
Jan 28, 2017
Merged

revise copy for intro array challenge#12817
dhcodes merged 1 commit intofreeCodeCamp:stagingfrom
no-stack-dub-sack:fix/array-copy-update

Conversation

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

@no-stack-dub-sack no-stack-dub-sack commented Jan 25, 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

  • Curriculum update
  • 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

This is just a quick PR to update the copy for the Array section of the basic data structure challenges. I've been working through these for the last few days, and one thing I noticed is that the Object challenges gave a bit better of an overview on objects as data structures, so just wanted to match the tone and scope of that conversation (given the topic of the section) to make sure the array intro was just as thorough and complete.

@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 Jan 25, 2017
@dhcodes dhcodes self-requested a review January 25, 2017 16:59
@dhcodes
Copy link
Copy Markdown
Contributor

dhcodes commented Jan 25, 2017

I think at least part of both this challenge and the Add-KeyValue-Pairs challenge should be converted into intro sections. There is a lot of text on these challenges and most of it doesn't actually have to do with the challenge exercise. Could this be split into a challenge and an intro like this?

cc @freeCodeCamp/moderators Thoughts?

@raisedadead
Copy link
Copy Markdown
Member

I agree with you @dhcodes , this is a bit wordy IMHO.

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

@dhcodes @raisedadead I think that's an excellent idea and was def getting concerned with the wordiness of it all. If this is an option, I'd gladly do it. I think the information is important, and adds context to the section (otherwise they just become more object and array challenges).

Only the most dedicated campers will probably end up reading it, but hey, at least no one can say we didn't provide the info

@bonham000
Copy link
Copy Markdown
Contributor

This looks good to me!

@HKuz
Copy link
Copy Markdown
Member

HKuz commented Jan 26, 2017

@no-stack-dub-sack - with the intro done for this section, this PR can close out #12629, too 👍

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

no-stack-dub-sack commented Jan 26, 2017

@HKuz awesome, thanks! Was actually going to go look and see if you had opened one for this section. Perfect! I just added that to the PR text

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

no-stack-dub-sack commented Jan 26, 2017

Ok, consulted w/ @bonham000 and made just a few more edits. @bonham000 if you want to take a look at this PR, let me know what you think. Based on @dhcodes & @raisedadead's suggestion I've modified to add an intro challenge to each section.

For the object section, I pulled the intro material from the first 2 challenges. I don't think this compromises the integrity of either challenge as I repeated the info much more succinctly there.

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

@raisedadead Same question on this one - once #12720 is merged, will I need to rebase and force update this branch for the all of these changes to merge smoothly? Thanks! You've been a huge help to me lately

@raisedadead
Copy link
Copy Markdown
Member

Yes, @no-stack-dub-sack and on that note may be the other PR #12720 can be taken up later. In which case we should merge this in and rebase that, which we are going to do in any case as it has conflicts.

Thanks and happy to help!

Copy link
Copy Markdown
Contributor

@dhcodes dhcodes left a comment

Choose a reason for hiding this comment

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

Some wording fixes and other additions.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

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

no-stack-dub-sack commented Jan 28, 2017

@dhcodes ok, so I made all the changes you mentioned except for one - see last "outdated" comment above. So I think this should be good. And, you pointing out a few typos gave me a strong hunch that there would be more, so I spellchecked the whole thing and caught several additional typos.

If you and @raisedadead are ok with all of this, I think this section should be in pretty sound shape!

@raisedadead
Copy link
Copy Markdown
Member

✨ ✨ LGTM. ✨ ✨

Thanks @no-stack-dub-sack

@dhcodes please have a look and merge.

This comment was marked as off-topic.

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

@dhcodes haha, no worries, nice catch yet again, better to get this right! All set

@dhcodes
Copy link
Copy Markdown
Contributor

dhcodes commented Jan 28, 2017

LGTM 👍 💯 🌮 Thanks for all the changes.

@dhcodes dhcodes merged commit f11f204 into freeCodeCamp:staging Jan 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 Jan 28, 2017
@no-stack-dub-sack no-stack-dub-sack deleted the fix/array-copy-update branch January 29, 2017 05:02
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.

6 participants