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

dagreader: remove Offset() method #5190

Merged
merged 1 commit into from
Jul 16, 2018
Merged

dagreader: remove Offset() method #5190

merged 1 commit into from
Jul 16, 2018

Conversation

schomatis
Copy link
Contributor

Remove Offset() from the DagReader interface. It's not part of the Unix API
and it wasn't used anywhere except for the tests (a helper function was added to
replace it).

Remove `Offset()` from the `DagReader` interface. It's not part of the Unix API
and it wasn't used anywhere except for the tests (a helper function was added to
replace it).

License: MIT
Signed-off-by: Lucas Molas <[email protected]>
@schomatis schomatis added need/review Needs a review topic/technical debt Topic technical debt topic/UnixFS Topic UnixFS labels Jul 5, 2018
@schomatis schomatis added this to the Files API Documentation milestone Jul 5, 2018
@schomatis schomatis self-assigned this Jul 5, 2018
@schomatis schomatis requested a review from Kubuxu as a code owner July 5, 2018 02:55
@ghost ghost added the status/in-progress In progress label Jul 5, 2018
@@ -750,3 +750,11 @@ func BenchmarkDagmodWrite(b *testing.B) {
}
}
}

func getOffset(reader uio.DagReader) int64 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm rewriting getOffset from dagreader_test.go, exporting it doesn't seem to work, is there a cleaner way (without adding it back to dagreader.go)?

@schomatis schomatis removed the status/in-progress In progress label Jul 5, 2018
Copy link
Member

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

yay, cleanup!

@schomatis schomatis removed the need/review Needs a review label Jul 9, 2018
@whyrusleeping whyrusleeping merged commit c9cda2c into ipfs:master Jul 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFM topic/technical debt Topic technical debt topic/UnixFS Topic UnixFS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants