8000 Pass start dir to moduleconfig by m-kusnierz · Pull Request #2 · debitoor/nodeerrors · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@m-kusnierz
Copy link
@m-kusnierz m-kusnierz commented Aug 14, 2023

Exported function in lib/nodeerrors.js instead of exporting value returned by the function.

I also had to modify moduleconfig package so it takes a directory it should check for config, because it was also being cached by opentelemetry package (so it was always looking for the config in the same directory).
Depends on debitoor/moduleconfig#2

@m-kusnierz m-kusnierz requested a review from a team August 14, 2023 08:59
message:"error %s",
args:["myParameter"]
it('will format the message with one argument and an internal parameter', function () {
const internalData = { test: true, myArray: [1, 2, 3] };

Choose a reason for hiding this comment

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

👍

Comment on lines -55 to +57
module.exports = getNodeErrors();
module.exports = getNodeErrors;

Choose a reason for hiding this comment

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

👍

package.json Outdated
"moduleconfig": "^2.0.0",
"underscore": "^1.8.3",
"uuid": "^3.0.1"
"@debitoor/moduleconfig": "github:debitoor/moduleconfig#pass-start-dir-to-function",
Copy link
Author

Choose a reason for hiding this comment

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

This will be updated to specific version after releasing debitoor/moduleconfig#2

"author": "Allan Ebdrup",
"name": "nodeerrors",
"author": "Debitoor",
"name": "@debitoor/nodeerrors",
Copy link

Choose a reason for hiding this comment

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

I would not do this - it will make you publish an all new package under this scope. In such case, you should use version 1 as the previous versions will not exist for @debitoor/nodeerrors - I'd recommend to keep it as is.

Copy link
Author

Choose a reason for hiding this comment

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

nodeerrors in npm registry is not owned by us, so I guess we won't be able to publish it with this name.
@debitoor/nodeerrors with version 1 sounds better then?

Copy link

Choose a reason for hiding this comment

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

@bifrost is a collaborator on https://www.npmjs.com/package/nodeerrors - you can ask him to add you 😄

@m-kusnierz m-kusnierz marked this pull request as ready for review August 17, 2023 06:36
@m-kusnierz m-kusnierz merged commit 946e0c3 into master Aug 17, 2023
@m-kusnierz m-kusnierz deleted the pass-start-dir-to-moduleconfig branch August 17, 2023 06:38
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.

3 participants

0