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.

Sign up for GitHub

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

Jump to bottom

feat: range request helpers #162

Merged
merged 20 commits into from
Oct 2, 2023
Merged

feat: range request helpers #162

merged 20 commits into from
Oct 2, 2023

Conversation

aschmahmann
Copy link
Contributor

cc @laurentsenta @lidel

aschmahmann
aschmahmann commented Sep 11, 2023
View reviewed changes
tooling/helpers/range.go Outdated Show resolved Hide resolved
tooling/test/sugar.go Outdated Show resolved Hide resolved
tooling/test/validate.go Show resolved Hide resolved
tooling/test/validate.go Outdated Show resolved Hide resolved
tooling/test/validate.go Show resolved Hide resolved
tooling/helpers/range.go Outdated Show resolved Hide resolved
@aschmahmann aschmahmann force-pushed the feat/range-request-helpers branch 4 times, most recently from db51bb2 to 73b949f Compare September 11, 2023 17:54
laurentsenta
laurentsenta reviewed Sep 12, 2023
View reviewed changes
tooling/test/sugar.go Outdated Show resolved Hide resolved
@BigLep BigLep requested a review from lidel September 12, 2023 16:48
@BigLep BigLep mentioned this pull request Sep 12, 2023
Merged
5 tasks
aschmahmann
aschmahmann commented Sep 19, 2023
View reviewed changes
tooling/test/test.go Outdated Show resolved Hide resolved
aschmahmann
aschmahmann commented Sep 19, 2023
View reviewed changes
tooling/helpers/range.go Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Sep 19, 2023 •
edited
Loading

Results against Kubo latest:
(check the action's summary for the full results)

Summary

Tests Failures Errors Skipped
112 1 0 0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 19, 2023 •
edited
Loading

Results against Kubo master:
(check the action's summary for the full results)

Summary

Tests Failures Errors Skipped
112 1 0 0

@aschmahmann
switch Response and HeaderBuilder Clones from doing shallow clones wh…
ed808db 
…en deep ones are not possible to panicking
@aschmahmann
Revert "switch Response and HeaderBuilder Clones from doing shallow c…
be3f9e7 
…lones when deep ones are not possible to panicking"

This reverts commit ed808db.
laurentsenta
laurentsenta approved these changes Sep 21, 2023
View reviewed changes
Copy link
Contributor

@laurentsenta laurentsenta left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The code looks mostly fine, feel free to take into account feedback or merge as is.

Could you add a line in the Changelog?

lidel
lidel previously requested changes Sep 21, 2023
View reviewed changes
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

I am not sure if we should merge this without cleaning up DSL first.
We may not have time to clean up later. See comments inline.

tooling/helpers/range.go Outdated Show resolved Hide resolved
tests/path_gateway_dag_test.go Outdated Show resolved Hide resolved
tooling/helpers/range.go Outdated Show resolved Hide resolved
tests/path_gateway_dag_test.go Show resolved Hide resolved
@hacdias hacdias assigned hacdias and unassigned aschmahmann Sep 25, 2023
laurentsenta
laurentsenta requested changes Sep 28, 2023
View reviewed changes
Copy link
Contributor

@laurentsenta laurentsenta left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I left a comment regarding values / pointer,
could you also add a line to the Changelog?

Everything else lgtm, thanks 🙏

@hacdias
Copy link
Member

hacdias commented Oct 2, 2023 •
edited
Loading

@laurentsenta I changed it to use values! Once this is approved, I will merge, and once it's released we can finally merge ipfs/boxo#369.

laurentsenta
laurentsenta reviewed Oct 2, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@laurentsenta laurentsenta self-requested a review October 2, 2023 09:05
laurentsenta
laurentsenta approved these changes Oct 2, 2023
View reviewed changes
Copy link
Contributor

@laurentsenta laurentsenta left a comment

Choose a reason for hiding this comment

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

lgtm!

@hacdias hacdias merged commit d7e8118 into main Oct 2, 2023
14 of 16 checks passed
@hacdias hacdias deleted the feat/range-request-helpers branch October 2, 2023 09:12
@github-actions github-actions bot mentioned this pull request Oct 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidel lidel lidel left review comments

@hacdias hacdias hacdias left review comments

@laurentsenta laurentsenta laurentsenta approved these changes

Assignees

@hacdias hacdias

Labels
None yet
Projects
No open projects
IPFS Shipyard Team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aschmahmann @hacdias @lidel @laurentsenta