Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@ejball
Copy link
Contributor

@ejball ejball commented Feb 17, 2019

Standard error codes have always been PascalCase, but I somehow failed to remember that when I initially wrote this code.

@ejball ejball requested a review from ddunkin February 17, 2019 01:10
@ejball
Copy link
Contributor Author

ejball commented Feb 17, 2019

Are package-lock.json files supposed to be committed?

@ejball
Copy link
Contributor Author

ejball commented Feb 17, 2019

I noticed the test error locally, but figured a PR was the easiest way to inquire about it.

@types/node version wasn't specified, and newer versions have a more return complicated type for http.Server.address(). This fixes an error building the tests.

6 and 8 are both currently maintenance LTS, but 6 is about to be EOL.

package-lock.json was generated from npm 6.8.0.
Copy link
Contributor

@ddunkin ddunkin left a comment

Choose a reason for hiding this comment

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

Should this be on the v1 branch? It is a breaking change, right?

Yes, package-lock.json should be committed.

@ejball
Copy link
Contributor Author

ejball commented Feb 18, 2019

It's a breaking bug fix, so I wondered if I should make it available to v1 users.

But I suppose we could only make it available to v2 users.

@ejball
Copy link
Contributor Author

ejball commented Feb 18, 2019

Thanks for figuring out the issue.

@ejball ejball merged commit a0ffe0a into FacilityApi:v1 Feb 22, 2019
@ejball ejball deleted the pascal-errors branch February 22, 2019 17:07
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.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.