981076 - [meta][Contacts] JSHint Fixes

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Description

[Kevin Grandon :kgrandon]

JSHint provides much greater linting capabilities than the current gjslint implementation. As gaia is migrating over to JSHint, we should consider moving contacts to use it. JSHint will also often help catch regressions and dead code paths.

Comment 1

[Kevin Grandon :kgrandon]

Do you guys mind if I start making fixes here? This would help a lot when doing bigger refactoring as it will provide both early regression detection and ensure that we don't land with unused code. This would be nice to have for bug 975680 for example.

I think this could be a meta bug, then we could open pull requests for various folders. E.g., js/views/*, and js/utilities/*, test/unit/*. (This is probably too much to do in one pass for one person, but should be more manageable if we split it up in this way). Please let me know what you think.

Comment 2

[Jose Manuel Cantera]

I'm ok with starting this work provided is done step by step and in controlled bugs and commits

Comment 3

[Francisco Jordano [:arcturus] [:francisco]]

Perfect!

Are you planning use this as a meta bug, or just add patches?

We could split the task to speedup. I'll have some cycles to give a hand. Should we sync in IRC?

Comment 4

[Kevin Grandon :kgrandon]

(In reply to Francisco Jordano [:arcturus] from comment #3)
> Perfect!
> 
> Are you planning use this as a meta bug, or just add patches?
> 
> We could split the task to speedup. I'll have some cycles to give a hand.
> Should we sync in IRC?

Perhaps we can take this as a meta-bug, and add blocker bugs to it based on sections/file groups that make sense? That way there will be a bunch of un-assigned bugs, and you/Jose/whoever can simply take them at will?

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Sounds good to me :)

Was checking a bit:

jshint apps/communications/contacts/ | awk '{print $1}' |  sort | uniq

Well we have a nice list:

apps/communications/contacts/js/contacts.js:
apps/communications/contacts/js/contacts_cleaner.js:
apps/communications/contacts/js/contacts_importer.js:
apps/communications/contacts/js/contacts_matcher.js:
apps/communications/contacts/js/contacts_matching_controller.js:
apps/communications/contacts/js/contacts_matching_ui.js:
apps/communications/contacts/js/contacts_tag.js:
apps/communications/contacts/js/export/bt.js:
apps/communications/contacts/js/export/contacts_exporter.js:
apps/communications/contacts/js/export/sim.js:
apps/communications/contacts/js/fb/datastore_migrator.js:
apps/communications/contacts/js/fb/fb_contact.js:
apps/communications/contacts/js/fb/fb_contact_utils.js:
apps/communications/contacts/js/fb/fb_data.js:
apps/communications/contacts/js/fb/fb_init.js:
apps/communications/contacts/js/fb/fb_link.js:
apps/communications/contacts/js/fb/fb_link_init.js:
apps/communications/contacts/js/fb/fb_messaging.js:
apps/communications/contacts/js/fb/fb_query.js:
apps/communications/contacts/js/fb/fb_utils.js:
apps/communications/contacts/js/fb/friends_list.js:
apps/communications/contacts/js/fb_loader.js:
apps/communications/contacts/js/fb_resolver.js:
apps/communications/contacts/js/import_init.js:
apps/communications/contacts/js/import_utils.js:
apps/communications/contacts/js/importer_ui.js:
apps/communications/contacts/js/navigation.js:
apps/communications/contacts/js/service_extensions.js:
apps/communications/contacts/js/sms_integration.js:
apps/communications/contacts/js/utilities/config.js:
apps/communications/contacts/js/utilities/confirm.js:
apps/communications/contacts/js/utilities/event_listeners.js:
apps/communications/contacts/js/utilities/http_rest.js:
apps/communications/contacts/js/utilities/icc_handler.js:
apps/communications/contacts/js/utilities/image_loader.js:
apps/communications/contacts/js/utilities/import_from_vcard.js:
apps/communications/contacts/js/utilities/import_sim_contacts.js:
apps/communications/contacts/js/utilities/misc.js:
apps/communications/contacts/js/utilities/overlay.js:
apps/communications/contacts/js/utilities/sdcard.js:
apps/communications/contacts/js/utilities/sim_dom_generator.js:
apps/communications/contacts/js/utilities/status.js:
apps/communications/contacts/js/utilities/templates.js:
apps/communications/contacts/js/value_selector.js:
apps/communications/contacts/js/views/details.js:
apps/communications/contacts/js/views/form.js:
apps/communications/contacts/js/views/list.js:
apps/communications/contacts/js/views/search.js:
apps/communications/contacts/js/views/settings.js:
apps/communications/contacts/oauth2/js/flow.js:
apps/communications/contacts/oauth2/js/oauth20.js:
apps/communications/contacts/test/marionette/activities_test.js:
apps/communications/contacts/test/marionette/details_test.js:
apps/communications/contacts/test/marionette/form_test.js:
apps/communications/contacts/test/marionette/lib/contacts.js:
apps/communications/contacts/test/marionette/lib/sms.js:
apps/communications/contacts/test/marionette/search_test.js:
apps/communications/contacts/test/unit/contacts_matcher_test.js:
apps/communications/contacts/test/unit/contacts_matching_ui_test.js:
apps/communications/contacts/test/unit/contacts_matching_ui_test_data.js:
apps/communications/contacts/test/unit/contacts_tag_test.js:
apps/communications/contacts/test/unit/datastore_migrator_test.js:
apps/communications/contacts/test/unit/export/bt_test.js:
apps/communications/contacts/test/unit/export/generic_export_test.js:
apps/communications/contacts/test/unit/export/sim_test.js:
apps/communications/contacts/test/unit/fb_data_test.js:
apps/communications/contacts/test/unit/fb_reader_utils_test.js:
apps/communications/contacts/test/unit/helper.js:
apps/communications/contacts/test/unit/import_vcard_test.js:
apps/communications/contacts/test/unit/link_test.js:
apps/communications/contacts/test/unit/mock_activities.js:
apps/communications/contacts/test/unit/mock_asyncstorage.js:
apps/communications/contacts/test/unit/mock_confirm_dialog.js:
apps/communications/contacts/test/unit/mock_contacts.js:
apps/communications/contacts/test/unit/mock_contacts_details.js:
apps/communications/contacts/test/unit/mock_contacts_exporter.js:
apps/communications/contacts/test/unit/mock_contacts_form.js:
apps/communications/contacts/test/unit/mock_contacts_index.html.js:
apps/communications/contacts/test/unit/mock_contacts_list.js:
apps/communications/contacts/test/unit/mock_contacts_list_obj.js:
apps/communications/contacts/test/unit/mock_contacts_match.js:
apps/communications/contacts/test/unit/mock_contacts_matching_controller.js:
apps/communications/contacts/test/unit/mock_contacts_search.js:
apps/communications/contacts/test/unit/mock_contacts_settings.js:
apps/communications/contacts/test/unit/mock_contacts_shortcuts.js:
apps/communications/contacts/test/unit/mock_cookie.js:
apps/communications/contacts/test/unit/mock_export_strategy.js:
apps/communications/contacts/test/unit/mock_extfb.js:
apps/communications/contacts/test/unit/mock_fb.js:
apps/communications/contacts/test/unit/mock_fb_data.js:
apps/communications/contacts/test/unit/mock_fb_loader.js:
apps/communications/contacts/test/unit/mock_find_matcher.js:
apps/communications/contacts/test/unit/mock_get_device_storage.js:
apps/communications/contacts/test/unit/mock_icc_helper.js:
apps/communications/contacts/test/unit/mock_l10n.js:
apps/communications/contacts/test/unit/mock_link.html.js:
apps/communications/contacts/test/unit/mock_linked_contacts.js:
apps/communications/contacts/test/unit/mock_matching_contacts.html.js:
apps/communications/contacts/test/unit/mock_mozActivity.js:
apps/communications/contacts/test/unit/mock_mozContacts.js:
apps/communications/contacts/test/unit/mock_navigation.js:
apps/communications/contacts/test/unit/mock_oauthflow.js:
apps/communications/contacts/test/unit/mock_sdcard.js:
apps/communications/contacts/test/unit/mock_search.js:
apps/communications/contacts/test/unit/mock_sim_importer.js:
apps/communications/contacts/test/unit/mock_utils.js:
apps/communications/contacts/test/unit/mock_value_selector.js:
apps/communications/contacts/test/unit/mock_wakelock.js:
apps/communications/contacts/test/unit/navigation_test.js:
apps/communications/contacts/test/unit/views/details_test.js:
apps/communications/contacts/test/unit/views/form_test.js:
apps/communications/contacts/test/unit/views/list_test.js:
apps/communications/contacts/test/unit/views/settings_test.js:

I see like several categories:
main scripts
views
test
utilities
import/export

Comment 6

[Kevin Grandon :kgrandon]

Nice. I would propose perhaps the following:

js/export
js/facebook
js/utilities
js/views
test/marionette
test/unit

I suppose there is always the option of making it more granular to make it more manageable. The system v2 change is actually going through and doing it file-by-file, though there they are doing refactoring and adding tests.

Comment 7

[Francisco Jordano [:arcturus] [:francisco]]

Closing as fixed since we mighty removed all jshint errors ;)

Comment 8

[Kevin Grandon :kgrandon]

Awesome! Thanks Jose and Francisco!