Conversation
fix lockfile property spelling and prevent racecondition for applicat…
Fixes for cert verification integration tests only (no auth middleware…
…parated Keep multiple set-cookie headers separate
…ass on query to subscribe
Ensure HARPER_SET_CONFIG can override any other config source
Fix mergeHeaders, where set-cookie headers could get combined, and se…
Have deploy-component fail by default if a conflicting config entry exists (can be forced)
* Limit message size so we don't get messages rejected on the receiving side * Add request body size limits * Add test of too large of request body * Respond to too large of request body more politely * Update server/serverHelpers/contentTypes.ts Co-authored-by: Chris Barber <chris@harperdb.io> --------- Co-authored-by: Chris Barber <chris@harperdb.io>
Mostly just because my editor puts red squigglies underneath them haha
This will come back once we get the unit testing infrastructure ready in this repo.
Ethan-Arrowood
left a comment
There was a problem hiding this comment.
generally LGTM but lets get CI happy first
.git-blame-ignore-revs
Outdated
| @@ -1,3 +1,4 @@ | |||
| # Format Commits | |||
| 84de82b25fff81f7b28c1a83789d1a9247fa73b4 | |||
There was a problem hiding this comment.
I don't know if order matters, but I believe I've been appending to this file so far
| "lint": "eslint .", | ||
| "lint:required": "eslint --no-config-lookup --quiet -c eslint.required.config.mjs .", |
There was a problem hiding this comment.
Is required referring to the no-undefined rule? Like why does this ruleset differ from the default one? When should I lint with lint vs lint:required?
We need those questions answered in contributing docs somewhere. Or we should be replacing the existing config with this one if we intend for it to be a check. My editor tells me about lint issues based on the default config, but doesn't pick this new one up. But we are using this new one for PR checks? It should be the one and only lint config then, right?
There was a problem hiding this comment.
As this is a sync PR, this is all explained in the original PR from the old repo: https://github.com/HarperFast/harperdb/pull/3013
However, I'm happy to add some notes about it to the contributing docs in here.
The short version is that nothing else needs to change, this is just to allow us to gradually start enforcing some linter rules in CI over time (starting with no-undef). But we want the larger ruleset to continue applying in all other contexts. Eventually there should only be one ruleset that is enforced everywhere.
I plan to try to enable one of the disabled rules in the required config every week going forward (along with the necessary fixes to make it pass).
There was a problem hiding this comment.
Unfortunately that original PR is not available for anyone but us. I know we don't exactly have public contributors yet, but I'm intentionally trying to force us to be transparent. Any information hidden in the old private repo that is relevant must be bubbled up.
And don't worry about adding to these sync PRs - thats the point of them. Its not just cherry-picking commits, but also making sure they all work with the OSS version. Additional commits is 100% okay and expected even if they have changes.
There was a problem hiding this comment.
And the info you added looks great. approving now! thank you for doing all of this.
There was a problem hiding this comment.
Unfortunately that original PR is not available for anyone but us. I know we don't exactly have public contributors yet, but I'm intentionally trying to force us to be transparent. Any information hidden in the old private repo that is relevant must be bubbled up.
And don't worry about adding to these sync PRs - thats the point of them. Its not just cherry-picking commits, but also making sure they all work with the OSS version. Additional commits is 100% okay and expected even if they have changes.
Yeah, I'm 100% on board with all that. I just wasn't sure if you were asking for yourself or on behalf of the community, so was trying to cover both bases. :)
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
No description provided.