Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix (curriculum): Adding regex to disregard any whitespaces in the basic-css challenge #38330

Merged
merged 2 commits into from Aug 31, 2020
Merged

fix (curriculum): Adding regex to disregard any whitespaces in the basic-css challenge #38330

merged 2 commits into from Aug 31, 2020

Conversation

hassaanp
Copy link
Member

@hassaanp hassaanp commented Mar 7, 2020

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 master branch of freeCodeCamp.
  • All the files I changed are in the same world language, for example: only English changes, or only Chinese changes, etc.

Closes #38329

@gitpod-io
Copy link

gitpod-io bot commented Mar 7, 2020 •
edited

@RandellDawson
Copy link
Member

@hassaanp As I stated in this other PR, let's wait until I get the CSS parser working, so that we can completely redo these challenge tests. The other one was a "one off" merge. I would prefer not to keep doing "one off" fixes, so we are not rewriting the tests more than once.

Thanks.

@RandellDawson RandellDawson added the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Mar 7, 2020
@camperbot camperbot added language: English scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. labels Mar 7, 2020
@raisedadead
Copy link
Member

I think we should have an umbrellas issue for the CSS Parser that you are working on. And lets list all these open one-off issues about CSS and we just take it up at one go once the testing infra is ready.

moT01
moT01 previously approved these changes Aug 30, 2020
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.

LGTM 🎉

I'm not sure of the status of that parser but I vote not to hold off these fixes. This one, in particular, fixes the problem and is ready to go. Also note that this only changes tests in the responsive web design section so it's fine to merge this.

@moT01 moT01 mentioned this pull request Aug 30, 2020
Closed
4 tasks
RandellDawson
RandellDawson suggested changes Aug 31, 2020
View reviewed changes
Copy link
Member

@RandellDawson RandellDawson left a comment

Choose a reason for hiding this comment

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

@hassaanp Please see my suggestion to fix the regex.

...lish/01-responsive-web-design/basic-css/use-attribute-selectors-to-style-elements.english.md Outdated Show resolved Hide resolved
@moT01 moT01 removed the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Aug 31, 2020
@raisedadead
Copy link
Member

Hey @RandellDawson if I remember correctly, Hassan was busy with their family with a newborn 🙂 , can we add these changes ourselves?

@RandellDawson
Copy link
Member

@moT01 Can you validate my suggested changes fix the issue I pointed out? If you agree, then go ahead and make an extra commit for them and then I can approve/merge this PR.

@moT01 @RandellDawson
Update curriculum/challenges/english/01-responsive-web-design/basic-c…
4b94310 
…ss/use-attribute-selectors-to-style-elements.english.md

Co-authored-by: Randell Dawson <5313213+RandellDawson@users.noreply.github.com>
@moT01 moT01 dismissed stale reviews from ghost and themself via 4b94310 August 31, 2020 21:24
moT01
moT01 approved these changes Aug 31, 2020
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.

I added your suggestion @RandellDawson, this LGTM 🎉

We can close #38576 if this gets merged since it fixes the same issue.

@RandellDawson RandellDawson merged commit b0f85de into freeCodeCamp:master Aug 31, 2020
@RandellDawson
Copy link
Member

Thank you for your contribution to the page! 👍
We're happy to accept these changes, and look forward to future contributions. 🎉

mehrdadaad
mehrdadaad reviewed Oct 26, 2020
View reviewed changes
Copy link

@mehrdadaad mehrdadaad left a comment

Choose a reason for hiding this comment

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

K

@mehrdadaad
Copy link

K

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

@mehrdadaad mehrdadaad mehrdadaad left review comments

@RandellDawson RandellDawson RandellDawson approved these changes

@moT01 moT01 moT01 approved these changes

Assignees
No one assigned
Labels
scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Broken Tests on challenge Basic CSS: Use Attribute Selectors to Style Elements
6 participants
@hassaanp @RandellDawson @raisedadead @mehrdadaad @moT01 @camperbot