Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove camel case ignore rule in ESLint #278

Merged
merged 12 commits into from
Apr 3, 2020

Conversation

cjchirag7
Copy link
Contributor

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

@birm birm requested review from nanli-emory and birm March 30, 2020 14:32
Copy link
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
Contributor Author

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

@birm
Copy link
Member

birm commented Mar 30, 2020

See

resetCallback: reset_callback,
for reset_callback and anno_callback

@cjchirag7
Copy link
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
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
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
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