[GCI] Standardised UI code comments #1345
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1345 +/- ##
=======================================
Coverage 66.12% 66.12%
=======================================
Files 125 125
Lines 2571 2571
Branches 404 404
=======================================
Hits 1700 1700
Misses 871 871
|
|
Kindly resolve conflicts. |
|
Resolved! |
Suggestion: Please review your PRs before they are reviewed by any mentor. This will save a lot of time. It will waste a lot of your time I will suggest you same changes again and again but you will do only some of those changes. Also, regarding the comment for asking mentors to be fast. I will say your pr is reviewed within 6 hours. This year GCI is EPIC so we are really fast. Thanks |
|
I was a bit worried because of school timings. It seems I will have some free time so no worries anymore. I won't ask anyone to be fast or anything.(I myself am not fast) |
|
Check online
…On Wed, 11 Dec 2019, 8:51 pm Harsh Khandeparkar, ***@***.***> wrote:
I was a bit worried because of school timings. It seems I will have some
free time so no worries anymore. I won't ask anyone to be fast or
anything.(I myself am not fast)
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#1345?email_source=notifications&email_token=AFAAEQ5R7BSNG6S3MTFXBH3QYEAOPA5CNFSM4JYKOIC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGTQE5I#issuecomment-564593269>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFAAEQYU47YNQEDYVRMDHE3QYEAOPANCNFSM4JYKOICQ>
.
|
|
Uh. Check what online? |
|
I can't believe, this issue had about a googol issues |
|
Ok. As you say. You are better qualified than me. I'll fix it in a min.
…On Thu, 12 Dec, 2019, 10:01 PM Sidharth Bansal, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In examples/demo.js
<#1345 (comment)>
:
> @@ -6,16 +6,16 @@ var defaultHtmlSequencerUi = require('./lib/defaultHtmlSequencerUi.js'),
insertPreview = require('./lib/insertPreview.js');
window.onload = function () {
- sequencer = ImageSequencer();
+ sequencer = ImageSequencer(); // Set the global sequencer variable.
I think it is a phrase as it doesn't have subject before
predicate(sentence = subject + predicate)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1345?email_source=notifications&email_token=AIJI5H57GRDCVAPK7UNHOSTQYJRMVA5CNFSM4JYKOIC2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPACSZQ#discussion_r357244538>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AIJI5H5WWVARVU2GCM6F7FLQYJRMVANCNFSM4JYKOICQ>
.
|
|
Fixed. |
|
Email replies are messed up. Github needs to fix this. |
There was a problem hiding this comment.
I have indicated all the places where . is inappropriate. If you are writing paragraph or sentence then only use full stops. Setting/Getting/Inserting doesn't require any full stop as they are phrases. If you are write function x is setting Y then you can use full stops.
Thanks
Small change will take 2-3 minutes. Lets complete it fast and merge it
examples/demo.js
Outdated
| var modulesInfo = sequencer.modulesInfo(); | ||
|
|
||
| var addStepSelect = $('#addStep select'); | ||
| addStepSelect.html(''); | ||
|
|
||
| // Add modules to the addStep dropdown | ||
| // Add modules to the addStep dropdown. |
There was a problem hiding this comment.
| // Add modules to the addStep dropdown. | |
| // Add modules to the addStep dropdown |
|
@jywarren resolved the conflicts. |
|
This one next! |
Fixes #1338 (<=== Replace
0000with the Issue Number)Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
npm test@publiclab/is-reviewersfor help, in a comment belowIf tests do fail, click on the red
Xto learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!