Add more details about rejecting console logs in tests #4780
Conversation
bajtos
added
developer-experience
|
|
||
| log('\n=== ATTENTION - INVALID USAGE OF CONSOLE LOGS DETECTED ===\n'); | ||
| log('\n=== ATTENTION - INVALID USAGE OF CONSOLE LOGS DETECTED ==='); | ||
| log('Learn more at http://ibm.biz/lb-mocha-console-logs\n'); |
There was a problem hiding this comment.
I think a GitHub permalink be better:
-
Docs are accurate for the specific version of
@loopback/buildThe docs may change over time to reflect functional changes. This means that the latest version of the docs may not be accurate for the version of
@loopback/buildthat is used by the user. -
The link can be updated when the docs are updated
This ties in with the first point - when the docs are updated, the link can be updated to the new permalink. This would ensure that the users are looking at the most accurate version of the docs.
-
Decoupling from IBM branding
This is more symbolic, but it'll provide assurance to the FOSS community that LoopBack 4 is and will continue to be an open-source project.
Regardless of IBM's commitment to open-source, some may still find comfort from the lack of Big Blue being mentioned.
Though I agree that seeing a long link isn't the prettiest:
As an alternative, maybe we could reference the local copy of the README instead? It'll satisfy all the requirements above.
There was a problem hiding this comment.
I prefer a github or loopback.io link too.
There was a problem hiding this comment.
+1 to a GitHub link. (Also, the current link adds a single quote at the end for me: https://github.com/strongloop/loopback-next/blob/master/packages/build/README.md#a-note-on-console-logs-printed-by-tests')
There was a problem hiding this comment.
Thank you for the feedback, I agree with your arguments and will change the link to point to GitHub master branch.
|
|
||
| log('\n=== ATTENTION - INVALID USAGE OF CONSOLE LOGS DETECTED ===\n'); | ||
| log('\n=== ATTENTION - INVALID USAGE OF CONSOLE LOGS DETECTED ==='); | ||
| log('Learn more at http://ibm.biz/lb-mocha-console-logs\n'); |
There was a problem hiding this comment.
+1 to a GitHub link. (Also, the current link adds a single quote at the end for me: https://github.com/strongloop/loopback-next/blob/master/packages/build/README.md#a-note-on-console-logs-printed-by-tests')
|
Thank you @achrinza @raymondfeng @nabdelgadir for valuable feedback ❤️ I added a commit to fix the issues, does the patch LGTY now? |
|
@bajtos Commit lint is failing - https://travis-ci.com/strongloop/loopback-next/jobs/294887891 |
It errored out so I restarted it and it's passing now 👍 It says the coverage dropped, but this PR isn't adding new code (besides one line of text). 😕 |
Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
bajtos
force-pushed
the
feat/improve-error-for-console-logs
branch
from
b598e86 to
e4d6fba
Compare
4 participants
As a follow-up to the conversation we had on Slack, I am proposing to add some documentation for
ATTENTION - INVALID USAGE OF CONSOLE LOGS DETECTEDand make this docs easier to discover by modifyinglb-mochato print a URL for that content.See also #1058 which introduced log detection to
lb-mocha.Checklist
👉 Read and sign the CLA (Contributor License Agreement) 👈
npm testpasses on your machinepackages/cliwere updatedexamples/*were updated👉 Check out how to submit a PR 👈