Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Refactor hooks api, plugins instantiation #9

Merged
merged 3 commits into from
May 31, 2019
Merged

Conversation

zephraph
Copy link
Member

No description provided.

@vercel
Copy link

vercel bot commented May 31, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://autobot-git-refactor-plugins.auto-it.now.sh

* Gives plugins an opportunity to do whatever it may need to do when PR processing
* is skipped. This is called _after_ the skip status has been set.
*/
onSkip: AsyncParallelHook<[PRContext, Config]>;
Copy link
Member Author

Choose a reason for hiding this comment

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

I found that this whole setup was unnecessary and was just created more issues than it was worth. Giving any random plugin the ability to pump the breaks on the whole process was a little... short sighted.


interface PluginCollection {
[pluginName: string]: UninitializedPlugin | InitializedPlugin;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Because PR plugins are instantiated in every PR I had issues where statuses were being send multiple times in dev because it's in server, not a lambda. I've essentially made sure that autobot will only ever have 1 instance of a Plugin at any given time.

@@ -111,16 +113,41 @@ export class Autobot {

private initializePlugins(scope: ExecutionScope, context?: PRContext) {
const scopePluginInstances = <T extends Plugin>() =>
this.plugins.filter(plugin => plugin.scope === scope).map(Plugin => new Plugin()) as T[];
Object.values(this.plugins)
.filter((options): options is UninitializedPlugin => !options.initialized)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a ts thing I learned tonight to get filter to return the type you want it to.

That's the :options is UninitializedPlugin part.

I really wish TS could handle this better generally. It was discussed here

cc @orta 😉


const logger = getLogger("autobot");
const STATUS_CONTEXT = isProduction ? "auto-bot" : "auto-bot-dev";
Copy link
Member Author

@zephraph zephraph May 31, 2019

Choose a reason for hiding this comment

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

This is so that people can test the dev version without it conflicting with the prod one. Basically can have two autobot apps installed at the same time. (Not that I recommend that of course)

if (this.failed) {
logger.info(`${formattedRepoName(context)} PR #${context.payload.number} missing required version labels`, {
url: context.url,
});
return {
state: "failure",
description: "Missing version or skip-release labels",
state: "pending",
Copy link
Member Author

Choose a reason for hiding this comment

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

I feel like pending is a better representation of the state of the pull request at the point when labels need to be added.

Failed makes it feel like something was done wrong and a change to code is needed. Pending just feels like it's waiting for something (like labels in this case).

What do you think @hipstersmoothie?

@zephraph zephraph self-assigned this May 31, 2019
@zephraph zephraph changed the title Refactor plugins Refactor hooks api, plugins instantiation May 31, 2019
@zephraph zephraph merged commit 487240f into master May 31, 2019
@zephraph zephraph deleted the refactor-plugins branch May 31, 2019 05:18
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.

1 participant