Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Switch to scope activationHook from language package #124

Merged
merged 6 commits into from
Feb 28, 2019
Merged

Switch to scope activationHook from language package #124

merged 6 commits into from
Feb 28, 2019

Conversation

vzamanillo
Copy link
Contributor

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

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

Super quick glance over this.

lib/linter-reek.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
When the package gets activated is controlled by the activationHook, however the package is still loaded on Atom startup. These should remain lazy loaded as before, however the deferred call in activate is no longer meaningful.
Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

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

Noticed one more issue with the code, and a reversion of one of the changes here after some local testing.

package.json Outdated Show resolved Hide resolved
lib/linter-reek.js Outdated Show resolved Hide resolved
Looks like the file itself (linter-reek.js) is only loaded when the activation hook is triggered... but it blocks the opening of the file until the package is done calling activate().

This means that all the optimizations we were doing before to reduce the load during Atom's initial startup are still useful for packages that use activationHooks because we don't want to be blocking the UI any more than we must.
Looks like this is listed as a supported scope, but wasn't listed as activating the package.
Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

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

Thanks for getting this project started!

@Arcanemagus Arcanemagus merged commit 1df159c into AtomLinter:master Feb 28, 2019
@vzamanillo vzamanillo deleted the use-activationhooks branch February 28, 2019 20:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants