[GCI] Standardised Core code comments #1343

harshkhandeparkar · 2019-12-09T14:49:59Z

Fixes #1340 (<=== Replace 0000 with the Issue Number)

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
code is in uniquely-named feature branch and has no merge conflicts
PR is descriptively titled
ask @publiclab/is-reviewers for help, in a comment below
Insert-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to 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!

codecov · 2019-12-09T14:54:01Z

Codecov Report

Merging #1343 into main will not change coverage.
The diff coverage is 75%.

@@           Coverage Diff           @@
##             main    #1343   +/-   ##
=======================================
  Coverage   66.41%   66.41%           
=======================================
  Files         125      125           
  Lines        2552     2552           
  Branches      397      397           
=======================================
  Hits         1695     1695           
  Misses        857      857

Impacted Files	Coverage Δ
src/modules/_nomodule/PixelManipulation.js	`77.77% <100%> (ø)`	⬆️
src/ImageSequencer.js	`50.3% <50%> (ø)`	⬆️

SidharthBansal · 2019-12-09T16:28:41Z

Please check code climate

harshkhandeparkar · 2019-12-09T16:30:18Z

Code climate errors are caused due to old code. This needs a dedicated pr to fix. This CC test is ignored in almost every pr.

…

On Mon, 9 Dec, 2019, 9:58 PM Sidharth Bansal, ***@***.***> wrote: Please check code climate — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1343?email_source=notifications&email_token=AIJI5H2EUIWURDU6TO4NIHTQXZW3XA5CNFSM4JYKNF72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGJZDGI#issuecomment-563319193>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIJI5H6WJFFKPNQEU3OSVZDQXZW3XANCNFSM4JYKNF7Q> .

src/ImageSequencer.js

src/modules/_nomodule/PixelManipulation.js

harshkhandeparkar · 2019-12-09T17:26:04Z

Omg. That is a breaking mistake. Btw, `Object` is a proper noun in JS.

…

On Mon, 9 Dec, 2019, 10:54 PM Sidharth Bansal, ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In src/ImageSequencer.js <#1343 (comment)> : > if (!sequences) { sequences = {}; - window.localStorage.setItem('sequences', JSON.stringify(sequences)); + window.localStorage.setItem('sequences', JSON.stringify(sequences)); // Set the localStorage entry as an empty Object by default. Object -> object ------------------------------ In src/ImageSequencer.js <#1343 (comment)> : > @@ -286,7 +377,7 @@ ImageSequencer = function ImageSequencer(options) { return { - //literals and objects + // Literals and objects. No need of full stop for phrases. Please do it for all prs ------------------------------ In src/ImageSequencer.js <#1343 (comment)> : > @@ -286,7 +377,7 @@ ImageSequencer = function ImageSequencer(options) { return { - //literals and objects + // Literals and objects. Use full stop for sentences ------------------------------ In src/modules/_nomodule/PixelManipulation.js <#1343 (comment)> : > @@ -106,7 +114,7 @@ module.exports = function PixelManipulation(image, options) { const inBrowser = (options.inBrowser) ? 1 : 0; const test = (process.env.TEST) ? 1 : 0; if (options.useWasm) { - if (options.inBrowser) { + if (options.inBroHawser) { You did some mistake here — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1343?email_source=notifications&email_token=AIJI5H5HH655IYGGWIGL6XDQXZ5MDA5CNFSM4JYKNF72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCOOXQHY#pullrequestreview-329087007>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIJI5HZVBZ6EBZBQE2YPO5DQXZ5MDANCNFSM4JYKNF7Q> .

harshkhandeparkar · 2019-12-09T17:26:20Z

I'll fix the phrases.

…

On Mon, 9 Dec, 2019, 10:55 PM M K, ***@***.***> wrote: Omg. That is a breaking mistake. Btw, `Object` is a proper noun in JS. On Mon, 9 Dec, 2019, 10:54 PM Sidharth Bansal, ***@***.***> wrote: > ***@***.**** requested changes on this pull request. > ------------------------------ > > In src/ImageSequencer.js > <#1343 (comment)> > : > > > if (!sequences) { > sequences = {}; > - window.localStorage.setItem('sequences', JSON.stringify(sequences)); > + window.localStorage.setItem('sequences', JSON.stringify(sequences)); // Set the localStorage entry as an empty Object by default. > > Object -> object > ------------------------------ > > In src/ImageSequencer.js > <#1343 (comment)> > : > > > @@ -286,7 +377,7 @@ ImageSequencer = function ImageSequencer(options) { > > > return { > - //literals and objects > + // Literals and objects. > > No need of full stop for phrases. > Please do it for all prs > ------------------------------ > > In src/ImageSequencer.js > <#1343 (comment)> > : > > > @@ -286,7 +377,7 @@ ImageSequencer = function ImageSequencer(options) { > > > return { > - //literals and objects > + // Literals and objects. > > Use full stop for sentences > ------------------------------ > > In src/modules/_nomodule/PixelManipulation.js > <#1343 (comment)> > : > > > @@ -106,7 +114,7 @@ module.exports = function PixelManipulation(image, options) { > const inBrowser = (options.inBrowser) ? 1 : 0; > const test = (process.env.TEST) ? 1 : 0; > if (options.useWasm) { > - if (options.inBrowser) { > + if (options.inBroHawser) { > > You did some mistake here > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#1343?email_source=notifications&email_token=AIJI5H5HH655IYGGWIGL6XDQXZ5MDA5CNFSM4JYKNF72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCOOXQHY#pullrequestreview-329087007>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AIJI5HZVBZ6EBZBQE2YPO5DQXZ5MDANCNFSM4JYKNF7Q> > . >

SidharthBansal

I think you missed to remove extra full stops from phrases. Kindly remove it too

src/ImageSequencer.js

SidharthBansal · 2019-12-12T03:42:33Z

src/ImageSequencer.js

  function loadNewModule(name, options) {

    if (!options) {
      return this;

    } else if (Array.isArray(options)) {
-      // contains the array of module and info
+      // Contains the array of module and info.


Many extra .
Please correct it

SidharthBansal · 2019-12-12T03:43:24Z

Kindly review prs once yourself too! It will be really helpful to you

src/ImageSequencer.js

harshkhandeparkar · 2019-12-12T14:48:48Z

I do review my PRs but I don't seem to find any mistakes. That is what reviewers are meant for I guess. Pointing out others' mistakes (Which I am very good at) 😝

SidharthBansal · 2019-12-12T16:29:08Z

Judge your code as if it was written by someone else

harshkhandeparkar · 2019-12-12T16:32:20Z

Yeah I know but it usually cannot happen on the same day. Like your mind is full and you need a bit of a break. I usually can self-review after 24 hours but asking a reviewer to review is better than waiting.

…

On Thu, 12 Dec, 2019, 9:59 PM Sidharth Bansal, ***@***.***> wrote: Judge your code as if it was written by someone else — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1343?email_source=notifications&email_token=AIJI5H6D2QI3Q3YO2HBZX2LQYJRFNA5CNFSM4JYKNF72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGXHGMA#issuecomment-565080880>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIJI5H4TLQXJIRMV6TMDGVDQYJRFNANCNFSM4JYKNF7Q> .

SidharthBansal

Kindly remove . wherever mentioned.
Thanks

src/ImageSequencer.js

src/modules/_nomodule/PixelManipulation.js

SidharthBansal

@jywarren kindly merge it
Thanks a lot Harsh!
Superb work. Let me know if any other doc issue needs to be reviewed or if you need approval for this gci task on the dashboard

jywarren · 2019-12-16T17:48:25Z

Resolved conflict and will merge when done! Thanks!

jywarren · 2019-12-16T17:56:07Z

Hooray!!

* standardise comments * fix * fixes * add docs links * remove extra . * ., this and other changes

Mr. X added 2 commits December 9, 2019 20:11

standardise comments

957153d

fix

cec7d56

harshkhandeparkar requested review from SidharthBansal and a team December 9, 2019 14:49

harshkhandeparkar changed the title ~~[GCI] Standardise Core code comments~~ [GCI] Standardised Core code comments Dec 9, 2019

harshkhandeparkar mentioned this pull request Dec 9, 2019

[GCI]Added More Comments to the Codebase #1335

Closed

5 tasks

SidharthBansal requested changes Dec 9, 2019

View reviewed changes

src/ImageSequencer.js Outdated Show resolved Hide resolved

src/ImageSequencer.js Outdated Show resolved Hide resolved

src/modules/_nomodule/PixelManipulation.js Outdated Show resolved Hide resolved

harshkhandeparkar and others added 2 commits December 10, 2019 17:03

Merge branch 'main' into core-code-comments

6c46764

fixes

41dc008

harshkhandeparkar requested review from SidharthBansal, jywarren and a team and removed request for jywarren December 10, 2019 17:06

SidharthBansal requested changes Dec 10, 2019

View reviewed changes

src/ImageSequencer.js Outdated Show resolved Hide resolved

src/ImageSequencer.js Outdated Show resolved Hide resolved

src/ImageSequencer.js Outdated Show resolved Hide resolved

add docs links

3e83f9e

SidharthBansal requested changes Dec 12, 2019

View reviewed changes

harshkhandeparkar commented Dec 12, 2019

View reviewed changes

src/ImageSequencer.js Outdated Show resolved Hide resolved

harshkhandeparkar commented Dec 12, 2019

View reviewed changes

src/ImageSequencer.js Outdated Show resolved Hide resolved

harshkhandeparkar commented Dec 12, 2019

View reviewed changes

src/ImageSequencer.js Outdated Show resolved Hide resolved

remove extra .

0c699bf

harshkhandeparkar requested a review from SidharthBansal December 12, 2019 14:47

Merge branch 'main' into core-code-comments

198d23c

SidharthBansal requested changes Dec 13, 2019

View reviewed changes

., this and other changes

392c6a6

harshkhandeparkar requested a review from SidharthBansal December 14, 2019 17:08

Merge branch 'main' into core-code-comments

97f2ba6

SidharthBansal approved these changes Dec 15, 2019

View reviewed changes

harshkhandeparkar added the ready-to-merge label Dec 15, 2019

harshkhandeparkar mentioned this pull request Dec 16, 2019

[GCI] InsertStep functionality added for loadImage step #1360

Merged

5 tasks

Merge branch 'main' into core-code-comments

0f070e4

jywarren approved these changes Dec 16, 2019

View reviewed changes

jywarren merged commit 426733f into publiclab:main Dec 16, 2019

jywarren pushed a commit that referenced this pull request Dec 16, 2019

[GCI] Standardised Core code comments (#1343)

03125d3

* standardise comments * fix * fixes * add docs links * remove extra . * ., this and other changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GCI] Standardised Core code comments #1343

[GCI] Standardised Core code comments #1343

harshkhandeparkar commented Dec 9, 2019 •

edited

Loading

codecov bot commented Dec 9, 2019 •

edited

Loading

SidharthBansal commented Dec 9, 2019

harshkhandeparkar commented Dec 9, 2019 via email

harshkhandeparkar commented Dec 9, 2019 via email

harshkhandeparkar commented Dec 9, 2019 via email

SidharthBansal left a comment

SidharthBansal Dec 12, 2019

SidharthBansal commented Dec 12, 2019

harshkhandeparkar commented Dec 12, 2019

SidharthBansal commented Dec 12, 2019

harshkhandeparkar commented Dec 12, 2019 via email

SidharthBansal left a comment

SidharthBansal left a comment

jywarren commented Dec 16, 2019

jywarren commented Dec 16, 2019

[GCI] Standardised Core code comments #1343

[GCI] Standardised Core code comments #1343

Conversation

harshkhandeparkar commented Dec 9, 2019 • edited Loading

codecov bot commented Dec 9, 2019 • edited Loading

Codecov Report

SidharthBansal commented Dec 9, 2019

harshkhandeparkar commented Dec 9, 2019 via email

harshkhandeparkar commented Dec 9, 2019 via email

harshkhandeparkar commented Dec 9, 2019 via email

SidharthBansal left a comment

Choose a reason for hiding this comment

SidharthBansal Dec 12, 2019

Choose a reason for hiding this comment

SidharthBansal commented Dec 12, 2019

harshkhandeparkar commented Dec 12, 2019

SidharthBansal commented Dec 12, 2019

harshkhandeparkar commented Dec 12, 2019 via email

SidharthBansal left a comment

Choose a reason for hiding this comment

SidharthBansal left a comment

Choose a reason for hiding this comment

jywarren commented Dec 16, 2019

jywarren commented Dec 16, 2019

harshkhandeparkar commented Dec 9, 2019 •

edited

Loading

codecov bot commented Dec 9, 2019 •

edited

Loading