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

1082 - Linting Performance #1101

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open

1082 - Linting Performance #1101

wants to merge 31 commits into from

Conversation

lumodon
Copy link

@lumodon lumodon commented Sep 19, 2017

Fixes #1082

Overview

Reduces over 400 linting warnings down to 12 linting warnings
Most coming from 'import/named' and 'prefer-default-export'
This changes imports and exports to be static
Also removes autoloader and replaces with static list of imports and exports for entire folder structure

Data Model / DB Schema Changes

N/A

Environment / Configuration Changes

Removes 'keymirror' and 'autoloader' from package.json

Notes

  1. Last few linting warnings couldn't be fixed to due to be used/imported dynamically
  2. lots of files still mix and match import and require.
  3. Some files have been eslint-disabled, particularly ones involving comments with "TODO" or "NOTE" - if planning on removing those comments in future will have to search them instead of relying on linter to find them.

Many changes from following branches into one
1. serafin-1082
1. serafin-1082-attempt-2
1. patrick-serafin-1082
1. jorge-serafin-1082
1. serafin-1082-attempt-3
1. merge-test
- as of this commit - all tests passing
Removes unneeded console log
because models is being generated dynamically
Ran full tests - all tests passing
Copy link
Collaborator

@heyheyjp heyheyjp left a comment

Choose a reason for hiding this comment

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

See tip on more concise exports from index.js files.

getRetrospectiveSurvey,
getRetrospectiveSurveyQuestion,
getUser,
getUserSummary,
Copy link
Collaborator

@heyheyjp heyheyjp Sep 21, 2017

Choose a reason for hiding this comment

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

Protip: can accomplish this in half the lines:

export {default as findChapters} from './findChapters'
export {default as findMembers} from './findMembers'
export {default as findPhaseSummaries} from './findPhaseSummaries'
export {default as findPhases} from './findPhases'
export {default as findProjects} from './findProjects'
export {default as findProjectsForCycle} from './findProjectsForCycle'
export {default as findRetrospectiveSurveys} from './findRetrospectiveSurveys'
export {default as findUsers} from './findUsers'
export {default as getChapter} from './getChapter'
export {default as getCycleById} from './getCycleById'
export {default as getCycleVotingResults} from './getCycleVotingResults'
export {default as getMemberById} from './getMemberById'
export {default as getProject} from './getProject'
export {default as getProjectSummary} from './getProjectSummary'
export {default as getProjectSummaryForMember} from './getProjectSummaryForMember'
export {default as getRetrospectiveSurvey} from './getRetrospectiveSurvey'
export {default as getRetrospectiveSurveyQuestion} from './getRetrospectiveSurveyQuestion'
export {default as getUser} from './getUser'
export {default as getUserSummary} from './getUserSummary'

Copy link
Author

Choose a reason for hiding this comment

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

Making this switch leads to 49 failing tests on my end. Have tried a variety of ways of exporting and have not found a workable solution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it. I'll have a look.

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.

Linter script performance enhancement
2 participants