-
Notifications
You must be signed in to change notification settings - Fork 378
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
refactor!: dynamically load releasers in ./releasers directory #439
Conversation
Codecov Report
@@ Coverage Diff @@
## master #439 +/- ##
===========================================
- Coverage 89.51% 79.51% -10.00%
===========================================
Files 32 40 +8
Lines 3643 4487 +844
Branches 314 334 +20
===========================================
+ Hits 3261 3568 +307
- Misses 381 918 +537
Partials 1 1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but please add the new enum value for simple
.
src/releasers/index.ts
Outdated
|
||
export function getReleaserNames(): string[] { | ||
return releasers.map(releaser => { | ||
console.info(releaser.releaserName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we still need this logging?
src/releasers/index.ts
Outdated
!file.name.match(/index\.js/) | ||
) { | ||
const obj = require(`./${file.name}`) as {[key: string]: typeof ReleasePR}; | ||
releasers.push(obj[Object.keys(obj)[0]]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, I would probably add them to an object instead, so that we could get one by accessing it by name directly and without a for
loop. But I'm OK either way.
This refactor pulls some additional logic to the
ReleasePR
class (it now hasreleaserName
which is used rather than anenum
for looking up a specific release type, andlookupPackageName
method, which was a feature being done in an ad hoc way for only Node.js).BREAKING CHANGE: this removers the
ReleaseType
enum
, in favor of a dynamically generated list of loaders.