Skip to content
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

feat: add n3-patch support to update-manager #624

Merged
merged 5 commits into from
Nov 18, 2023

Conversation

damooo
Copy link
Contributor

@damooo damooo commented Oct 5, 2023

Closes #473

Also resolves SolidOS/solidos#185

@damooo
Copy link
Contributor Author

damooo commented Oct 5, 2023

Few style changes regarding spaces are done by my editor's autofmt. As there seems no eslint/prettier rules setted up, i hope it is ok?

src/update-manager.ts Outdated Show resolved Hide resolved
src/update-manager.ts Outdated Show resolved Hide resolved
src/update-manager.ts Show resolved Hide resolved
tests/unit/update-manager-n3patch-insert-test.ts Outdated Show resolved Hide resolved
src/update-manager.ts Show resolved Hide resolved
@angelo-v
Copy link
Contributor

angelo-v commented Oct 6, 2023

Thank you for adding this important feature!

@timbl
Copy link
Member

timbl commented Oct 8, 2023

Strange the removal of whitespace before defining parens, which eslint on other projects would add.

@damooo
Copy link
Contributor Author

damooo commented Oct 8, 2023

Strange the removal of whitespace before defining parens, which eslint on other projects would add.

@timbl

It follows airbnb's js styleguide. As in https://github.com/airbnb/javascript/blob/11ab37144b7f846f04f64a29b5beb6e00d74e84b/packages/eslint-config-airbnb-base/rules/style.js#L486

@damooo
Copy link
Contributor Author

damooo commented Nov 15, 2023

Sorry for not working on this. I was working on many things related to Manas. Now i pushed with tests that tests for complex cases with blank nodes. May be mergable now. Could add many in future. @timbl @angelo-v

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Minor, human-facing

src/update-manager.ts Outdated Show resolved Hide resolved
src/update-manager.ts Outdated Show resolved Hide resolved
src/update-manager.ts Outdated Show resolved Hide resolved
src/update-manager.ts Outdated Show resolved Hide resolved
damooo and others added 2 commits November 16, 2023 15:00
@damooo damooo requested a review from angelo-v November 16, 2023 09:40
@damooo
Copy link
Contributor Author

damooo commented Nov 18, 2023

Is there anything else i should address?

thanks.

@angelo-v angelo-v merged commit 2f2a2f3 into linkeddata:main Nov 18, 2023
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.

Support N3 Patch Accept accept-patch header as well as MS_Author_via
4 participants