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.

Sign up for GitHub

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

Plugin Classes gain name of constructor #485

Merged
merged 2 commits into from
Nov 30, 2020
Merged

Plugin Classes gain name of constructor #485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1723f6b
ismplify plugin runner
evantahler Nov 30, 2020
77e6aba
plugin classes get name of constrocutor
evantahler Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export abstract class Plugin {
};

constructor(worker, func, queue, job, args, options) {
this.name = "CustomPlugin";
this.name = this?.constructor?.name || "Node Resque Plugin";
this.worker = worker;
this.queue = queue;
this.func = func;
Expand Down
20 changes: 5 additions & 15 deletions src/core/pluginRunner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,16 @@ export async function RunPlugins(
args,
pluginCounter?
) {
if (!pluginCounter) {
pluginCounter = 0;
}
if (!job) {
return true;
}
if (!pluginCounter) pluginCounter = 0;
if (!job) return true;
Copy link
Contributor

@glensc glensc Dec 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

my coding style is never to do such "oneliners". always use braces for code blocks.

it keeps a consistent style, avoids bugs.

if you feel like the optimization makes code shorter, perhaps the function is too long, has too many responsibilities, and should be refactored by method extraction, etc. removing braces, does not make the code simpler.

also, for code minifying there's automated tools, no need to obfuscate code manually.

my 2ç

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your feedback!

I generally disagree, coming from a Ruby background. In my experience, folks tend to be able to be able to parse the logic of one liners more easily, as long as they are simple (return or set a value). The bounds of this conversation and the complexity of what would be allowed in 1-liners is checked be Prettier (Linter), which is comfortable with "short" 1-liners.

Copy link
Contributor

Choose a reason for hiding this comment

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

Spotted micro optimization, these lines should be swapped:

  if (!pluginCounter) pluginCounter = 0;
  if (!job) return true;

there's no point sanitize pluginCounter if next line returns from the method.

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly the kind of thing that's easier to see with these one-liners 👍

Lines swapped @ 664b58d

Copy link
Contributor

Choose a reason for hiding this comment

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

thought to share this: https://blog.codecentric.de/en/2014/02/curly-braces/

if (
job.plugins === null ||
job.plugins === undefined ||
job.plugins.length === 0
) {
return true;
}
if (pluginCounter >= job.plugins.length) {
return true;
}
if (pluginCounter >= job.plugins.length) return true;

const pluginRefrence = job.plugins[pluginCounter];
const toRun = await RunPlugin(
Expand All @@ -35,9 +29,7 @@ export async function RunPlugins(
args
);
pluginCounter++;
if (toRun === false) {
return false;
}
if (toRun === false) return false;

return RunPlugins(self, type, func, queue, job, args, pluginCounter);
}
Expand All @@ -51,9 +43,7 @@ export async function RunPlugin(
job,
args
) {
if (!job) {
return true;
}
if (!job) return true;

let pluginName = PluginRefrence;
if (typeof PluginRefrence === "function") {
Expand Down