Skip to content

refactor(gw/dir-index-html): remove gobindata#8872

Merged
Jorropo merged 3 commits intomasterfrom
remove-go-bin-data
Apr 12, 2022
Merged

refactor(gw/dir-index-html): remove gobindata#8872
Jorropo merged 3 commits intomasterfrom
remove-go-bin-data

Conversation

@lidel
Copy link
Member

@lidel lidel commented Apr 11, 2022

This is code by @ frankywahl from #8834 pushed as upstream branch just to fix Circle CI.

@lidel
Copy link
Member Author

lidel commented Apr 11, 2022

@Jorropo will you have time to fix errors detected on CI?

@lidel lidel added this to the Best Effort Track milestone Apr 11, 2022
@lidel lidel marked this pull request as ready for review April 11, 2022 19:53
Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

LGTM (altho go.mod needs a fix first)

@Jorropo
Copy link
Contributor

Jorropo commented Apr 11, 2022

@lidel I can tomorrow

@frankywahl
Copy link
Contributor

frankywahl commented Apr 11, 2022

LGTM (altho go.mod needs a fix first)

Right - xxhash is now a dependency - I missed that. Ran go mod tidy here - I think that is probably the missing dependency.

@Jorropo
Copy link
Contributor

Jorropo commented Apr 12, 2022

@lidel we actually use github.com/cespare/xxhash not github.com/OneOfOne/xxhash so I've changed it to compile a single implementation and don't make the binary bigger:

$ ipfs version deps | grep xxhash
github.com/cespare/[email protected]
github.com/cespare/xxhash/[email protected]

frankywahl and others added 2 commits April 12, 2022 02:18
Since go1.16, there are built in tools that allow for embeding
filesystem inside the binary. We now make use of the `embed` package to
have all files put into the binary, removing the need to generate the
files and removes dependencies

Co-authored-by: Jorropo <[email protected]>
This removes the delegation to the function and requires all callers
that used the `asset.Asset` func to access to asset via the `embed.FS`
@Jorropo Jorropo force-pushed the remove-go-bin-data branch from 201f4ce to 2a545c9 Compare April 12, 2022 00:20
@Jorropo Jorropo enabled auto-merge (rebase) April 12, 2022 00:20
@Jorropo
Copy link
Contributor

Jorropo commented Apr 12, 2022

I've also took the liberty to meaning fully squash thoses commits following atomic principles.

@Jorropo Jorropo merged commit ca4a3ed into master Apr 12, 2022
@Jorropo Jorropo deleted the remove-go-bin-data branch April 12, 2022 00:36
@lidel
Copy link
Member Author

lidel commented Apr 12, 2022

@Jorropo next time just squash an entire cleanup like this into a single commit – less noise, more signal in https://github.com/ipfs/go-ipfs/commits/master 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants