Skip to content

Standard error codes should use PascalCase. - #16

Merged
ejball merged 4 commits into
FacilityApi:v1from
ejball:pascal-errors
Feb 22, 2019
Merged

Standard error codes should use PascalCase.#16
ejball merged 4 commits into
FacilityApi:v1from
ejball:pascal-errors

Conversation

@ejball

@ejball ejball commented Feb 17, 2019

Copy link
Copy Markdown
Contributor

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

ejball commented Feb 17, 2019

Copy link
Copy Markdown
Contributor Author

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

@ejball

ejball commented Feb 17, 2019

Copy link
Copy Markdown
Contributor Author

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.

@ddunkin ddunkin left a comment

Copy link
Copy Markdown
Contributor

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

ejball commented Feb 18, 2019

Copy link
Copy Markdown
Contributor Author

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

ejball commented Feb 18, 2019

Copy link
Copy Markdown
Contributor Author

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