fix(challenges): fix test in hr challenge #189
Conversation
the current test never matches: in the html, </h4> is followed by <p>, not <em>
There was a problem hiding this comment.
@cellux LGTM thanks for your contribution.
@raisedadead one of the changes (#181) released yesterday has broken a test on one of the challenges. I quickly skimmed over the other changes in the PR and dont think it adversely affected anything else.
|
Hi @johnkennedy9147 do we know of what line needs to be updated? I can do a quick release based on that. |
|
Perfect. I'll get this released. |
## [3.1.2](v3.1.1...v3.1.2) (2018-07-31) ### Bug Fixes * **challenges:** allow for omitted unit after zero values ([45b573b](45b573b)), closes [#166](#166) * **challenges:** changed complementary color for blue to orange ([e41f078](e41f078)), closes [#17934](https://github.com/freeCodeCamp/curriculum/issues/17934) * **challenges:** fix test in hr challenge ([#189](#189)) ([2edb306](2edb306)) * **challenges:** fix third test for template literals ([b8d004e](b8d004e)), closes [#135](#135) * **challenges:** fixes escaping of ([fd8c9e4](fd8c9e4)) * **scripts:** fix unpack and repack scripts for the new challenge schema ([52ed7cf](52ed7cf))
|
🎉 This PR is included in version 3.1.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
It is still not released on the website: I have just added an issue for that: |
|
@raisedadead Is there an ETA for this fix to be rolled into the production site? I saw that it was committed back on July 28th. I am surprised this has not been released to production. How do these releases work? What determines when a committed fix will go into production? I am trying to understand the process. I am wanting to help out, by starting to work on a few issue items, but I would like to have a better understanding of the flow of how things go from being committed vs. being released. Any insight you can provide would be great. Thank you. |
|
Apologies about the delay. Unfortunately some schema changes have broken the tests on the main repository. More details are available on #204 We are working on a way to get these conflicts addressed. But until that happens I am afraid the fixes can't be rolled to the site as it is. That said please be rest assured that we working on this on priority. |
|
@RandellDawson just to address your queries:
Currently, we are publishing a npm package every few days on this repository. That happens when everything from Once a package is published, we are updating the same on both Finally, once everything is ready on both the repos (i.e, tests look good). Then we go in and do a deployment (which is a DevOps step by freeCodeCamp team) on the actually servers. We are trying to re-architecture a bit of this flow, at the minute to avoid schema conflicts as notes in the linked issue, and hence avoid the blockage. |
|
I am not sure what skillset I would need to begin to help you out for this current issue raised in #204, but if you can let me know, then I would be happy to attempt something. We are getting slammed on the forum with that one particular challenge. I also noticed some actually modified the Guide to reflect the following:
The really sad part about the above Guide reference is it actually suggests using a "hack" by another camper to pass the challenge tests. That "hack" promotes invalid html syntax. I want to help eliminate the need for the FCC Guide to suggest hacks for solutions to the challenges. Out of curiosity, why weren't the changes from #127 rolled back, once it was known to break the curriculum? |
|
Can the suggestion for the “hack” be removed from the Guide? If it can’t, can’t a moderator edit that linked post on the message board so that either a warning about it being invalid HTML is added, or the whole post is deleted and replaced with something like: SpaceCatInd found a way to alter the HTML to get the horizontal rule test to “pass”, and tried to help everyone else by sharing their workaround here. Sadly though, the workaround uses invalid HTML, which you shouldn’t write as it can cause problems with real webpages. So we’ve removed the details of the workaround. The problem will eventually be fixed. In the meantime though, please skip the test by clicking on “curriculum” at the top of the page, and then click on “Applied Visual Design” and start on the next test “Adjust the background-color Property of Text”. You’ll then be able to carry on with the other challenges as normal. Please note that you do not need to have completed every challenge in order to claim certificates or complete projects later on, so it will not hinder your progress through the course. |
|
We have just prepared pull requests: freeCodeCamp/freeCodeCamp#18160 These will be deployed soon, these fix the blocking issue of the broken tests. These also have all the latest changes to the curriculum with a freshly published package, can you or someone else dive in and guide what else needs to be done to fix the challenge in discussion.? If the instructions are to be modified? Or they are already done in the code and just need to be deployed? Correct? |
|
P.S: If you have any issue no.s that we have already, it'd be awesome for me to track all this myself. |
@raisedadead I will take a look at this particular challenge (HR element challenge) for sure. What is the easiest way for me to review other challenges which will be deployed in the same release? I don't mind reviewing all of them, but I am not as familiar with how to do that on Github, so I could use a point in the right direction/method. Thanks for getting all this ready go! |
|
@RandellDawson, here's the changelog with all of the challenges that have been updated or added for each release. Thank you for helping out with the guide! Would be best to get the hack/invalid method out of there as soon as we can. |
|
@scissorsneedfoodtoo So how many versions should I go back to review? I ask, because it is my understanding that the curriculum has not been able to update for quite some time. |
|
@RandellDawson, the current version used on Learn is 3.1.1, so I'm thinking just 3.1.2 and the latest 3.2.0 will need to be reviewed. Here are a few changes from 3.1.2 and 3.2.0 that stand out and might require changes in the guide:
Also, there was a new challenge added in the last release:
|
|
@raisedadead Sorry, but I have one last question. If I do find anything that might need to be changed, should I make a comment on the issue # referenced in the changelog or in this particular thread? I had a question about the fix implemented for one of the issues I originally raised. |
|
@raisedadead - It took me a while, but I reviewed many of the fixes put into place in the releases 3.12 and 3.2.0. Below is the list. I marked either OK if everything looked fine to me or DID NOT REVIEW. If you would prefer me to delete this large comment let me know. The only change I reviewed which seemed not to have all of the suggestions implemented was (ded4705, closes #[199 ]. RELEASE 3.2.0 (2018-09-20) Features RELEASE 3.1.2 (2018-07-31) |
|
Thanks for taking the time to review all these:
IMHO, we don't want it to be optional. This has been discussed time and again on the main repo, and I totally understand the desire to have the flexibility. However, just to re-iterate a majority of the users are new to coding, and I think its best that they are made aware of the nuances of a language like JS, which while being OK with not having semi-colons, and be messy to debug if, you have a bug, that is pure syntax oriented. Here is an article you can refer to for more insight |
|
Side note: I have just deployed the latest v3.2.0 to production. Please feel free to redirect fresh bugs as issues on the main repo. |
|
I agree with enforcing semicolons in JavaScript and not making them optional. If you’ve never had to wade through hundreds of lines of JS code to track down a bug, in one solitary browser, to discover all you needed to do was add in a semicolon at the end of a line... just no. Newbies will see it like CSS anyway; you need the semicolons there too. |
The “hr” test now passes OK for me, but there’s still reference to the hacky “workaround” in the “get a hint” area. Is there already an issue somewhere to take care of removing that? (I am somewhat confused as to the different repos and what issues are appropriate where.) |
|
@RaspberryLime please feel free to open a PR if you would be interested to remove the instructions on this repository. Issues maybe open on https://github.com/freeCodeCamp/freeCodeCamp/issues, but aren't necessary if the change is decided to be made anyways. You can search for the string on the top of this page to look for the file that needs modification. Thanks |
|
@raisedadead Since the Get a hint just links to the Guide which is what actually needs updating, wouldn't the issue need to be created at https://github.com/freeCodeCamp/guide/issues ? @RaspberryLime I just posted a reply over at freeCodeCamp/guide#8341 to see if we need to create another issue to get it removed from the guide, |
|
Hi @RandellDawson thanks a lot for pointing me to the location I have prepared freeCodeCamp/guide#8668 Can you please take a quick moment and let me know if it looks good? |
|
@RandellDawson and @raisedadead thank you! I was going to try and work out how to start a PR but I’ll try that another time. |
@raisedadead Sorry for not letting you know sooner, but I was offline for a couple of days. It looks fine to me. Go with it. Thanks for your help. Hopefully, there can be some kind of policy decision on whether or not workarounds you ever be in the Guide. @QuincyLarson - What are your thoughts on this issue? |
|
I would say workarounds are OK if they’re valid HTML/CSS or whatever. Teaching a second correct way of doing something isn’t a bad thing. But illegally nesting HTML tags? No. |
I agree. |
|
Hi @RandellDawson all the fixes are live now, and the guide has be updated to remove the instructions in the question. Since the discussion here is beyond the scope of this PR itself. Let's discuss any additional topics on the forum or a separate Issue on the main repo as you feel suitable? Thanks. |
|
No problem. |
The current test never matches: in the html,
</h4>is followed by<p>, not<em>Description
Pre-Submission Checklist
devbranch.fix/,feature/, ortranslate/(e.g.fix/challenge-tests)npm test.npm run committo generate a conventional commit message.Learn more here: https://conventionalcommits.org/#why-use-conventional-commits
If they were done on the web interface you have ensured that you are creating conventional commit messages.
Checklist:
Closes #17944