Skip to content

Remove camel case ignore rule in ESLint#278

Merged
birm merged 12 commits intocamicroscope:developfrom
cjchirag7:remove_camel_case
Apr 3, 2020
Merged

Remove camel case ignore rule in ESLint#278
birm merged 12 commits intocamicroscope:developfrom
cjchirag7:remove_camel_case

Conversation

@cjchirag7
Copy link
Copy Markdown
Contributor

Fixes a subissue of #274
I have made sure that the application works, as earlier.
@birm , please review.

@birm birm requested review from birm and nanli-emory March 30, 2020 14:32
Copy link
Copy Markdown
Member

@birm birm left a comment

Choose a reason for hiding this comment

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

still combing through, but you need to change anno_delete at

callback: anno_delete,
to annoDelete

@cjchirag7
Copy link
Copy Markdown
Contributor Author

@birm , @nanli-emory , the required changes have been made

@birm
Copy link
Copy Markdown
Member

birm commented Mar 30, 2020

See

resetCallback: reset_callback,
for reset_callback and anno_callback

@cjchirag7
Copy link
Copy Markdown
Contributor Author

@birm thanks for the review ! I checked all the html and js files once again and found some more changes to be required in table.html and init.js. I have made the required changes. I think now it should be fine.

cjchirag7 and others added 6 commits March 30, 2020 23:10
merge request camicroscope#278
variable name changing ====> `_target_viewer` => `_targetViewer`.
1. `toolbar._main_tools` => `toolbar._mainTools`, there are 8 places in `apps/veiwer/uicallbacks.js` that need to change when you change the variable`_main_tools` to `_mainTools`  in `components/toolbar/toolbar.js`. [line 17, 23, 1053, 1079, 1092, 1096, 1215, 1221]
2. `toolbar._sub_tools` => `toolbar._subTools`, there are 3 places in `apps/segment/segment.js`(2 places - line 850, 851 removed), `apps/labeling/labeling.js` (1 place - line 84) that need to change when you change the variable`_sub_tools` to `_subTools`  in `components/toolbar/toolbar.js`.
`toolbar._sub_tools` => `toolbar._subTools`
`toolbar._sub_tools` => `toolbar._subTools`
Copy link
Copy Markdown
Member

@nanli-emory nanli-emory left a comment

Choose a reason for hiding this comment

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

@birm I approved the merge but I'm not confident that we have found and changed all variables to camel case since this changing involves in a large scale of files. I'm afraid that some uncaught code will crush some functionalities. 😥

@cjchirag7
Copy link
Copy Markdown
Contributor Author

@birm , I have updated this PR to resolve merge conflicts. Please review.

@birm birm self-requested a review April 3, 2020 15:17
Copy link
Copy Markdown
Member

@birm birm left a comment

Choose a reason for hiding this comment

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

Smoke test looks ok.

@birm birm merged commit b0509cf into camicroscope:develop Apr 3, 2020
@birm birm mentioned this pull request Apr 3, 2020
@cjchirag7 cjchirag7 deleted the remove_camel_case branch April 3, 2020 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants