Add s5cmd. #18306

benjaminrwilson · 2022-03-12T04:58:14Z

Checklist

Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
License file is packaged (see here for an example).
Source is from official source.
Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
If static libraries are linked in, the license of the static library is packaged.
Build number is 0.
A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
When in trouble, please check our knowledge base documentation before pinging a team.

conda-forge-linter · 2022-03-12T04:58:17Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/s5cmd) and found it was in an excellent condition.

benjaminrwilson · 2022-03-12T05:30:41Z

@conda-forge/staged-recipes Ready for review.

recipes/s5cmd/meta.yaml

benjaminrwilson · 2022-03-14T16:05:26Z

@conda-forge/staged-recipes Fixes have been made and the PR is ready for review again.

benjaminrwilson · 2022-03-16T15:48:29Z

@conda-forge/staged-recipes Friendly ping on this :-)

benjaminrwilson · 2022-03-17T14:58:22Z

@chrisburr, would you mind taking a look at this if you get a chance?

benjaminrwilson · 2022-03-21T15:05:11Z

@BastianZim, would you mind taking a look at this?

BastianZim

Hi @benjaminrwilson sorry that no one reviewed your PR. We generally don't have too many go packages so there aren't many reviewers around. To be honest, go is outside my area of expertise but I have added some small comments.
The main problem here is the licensing but it seems like you solved that. Just one question here: Did you only include the licenses of the repos or also of the golang.org packages etc.?

recipes/s5cmd/meta.yaml

BastianZim · 2022-03-21T16:57:43Z

recipes/s5cmd/meta.yaml

+    - s5cmd --help
+
+about:
+  home: https://github.com/peak/{{ name }}


This is not a blocker but it would be good to hardcode this instead of using a variable. Sometimes the bot gets tripped up on those.

Thanks for taking a look, @BastianZim --- it's very appreciated!

Just to check, have you analysed the licenses with https://github.com/google/go-licenses as well? That generally gives a good overview.

That's correct.

This is not a blocker but it would be good to hardcode this instead of using a variable. Sometimes the bot gets tripped up on those.

Will push a fix!

That's correct.

Ok great! Would you recommend it for other packages as well? I'd add it to the docs then because the go licensing is an ongoing discussion.

Yeah, it went pretty smoothly. It required some manual effort of cleaning out the exported data (some lingering files unrelated to the actual license/notice files), but it's definitely much better than manually searching for everything.

Ok, that's good to know because for rust we have a tool that just does this during build so we might be able to do it here as well.

Also, variables are for things that change. We, hope, that the name of a package doesn't change :-). Using the package name instead of a variable makes the recipe more readable and easier to type too. Thanks for changing it!

recipes/s5cmd/meta.yaml

BastianZim · 2022-03-21T18:09:35Z

@xhochy As you're more familiar with go would you mind having a final look?

BastianZim · 2022-03-21T18:17:17Z

Let's give Uwe some time to review and if not I'll merge it.

BastianZim · 2022-03-24T18:43:44Z

I'll merge this once the GitHub Actions are fully back up, latest tomorrow.

BastianZim · 2022-03-24T19:54:43Z

@ocefpaf Since you opened a PR (#18339) for the same package two small questions:

You called yours go-s5cmd. Is that agreed upon now? I know we're doing it for R.
You had a couple more flags for building, should they be included?

ocefpaf · 2022-03-26T15:51:41Z

You called yours go-s5cmd. Is that agreed upon now? I know we're doing it for R. You had a couple more flags for building, should they be included?

I was following another recipe. To be honest I had no idea what to do with go. The prefixing is a good idea but it is not a standard nor it was discussed with the core members before. @isuruf can you provide some guidance and best practices for go recipes here?

isuruf · 2022-03-30T16:38:58Z

recipes/s5cmd/meta.yaml

+    - library_licenses/aws/aws-sdk-go/internal/sync/singleflight/LICENSE
+    - library_licenses/aws/aws-sdk-go/LICENSE.txt
+    - library_licenses/aws/aws-sdk-go/NOTICE.txt
+    - library_licenses/cpuguy83/go-md2man/v2/md2man/LICENSE.md
+    - library_licenses/davecgh/go-spew/spew/LICENSE
+    - library_licenses/hashicorp/errwrap/LICENSE
+    - library_licenses/hashicorp/go-multierror/LICENSE
+    - library_licenses/jmespath/go-jmespath/LICENSE
+    - library_licenses/karrick/godirwalk/LICENSE
+    - library_licenses/kballard/go-shellquote/LICENSE
+    - library_licenses/peak/s5cmd/LICENSE
+    - library_licenses/pmezard/go-difflib/difflib/LICENSE
+    - library_licenses/posener/complete/LICENSE.txt
+    - library_licenses/russross/blackfriday/v2/LICENSE.txt
+    - library_licenses/shurcooL/sanitized_anchor_name/LICENSE
+    - library_licenses/stretchr/objx/LICENSE
+    - library_licenses/stretchr/testify/LICENSE
+    - library_licenses/termie/go-shutil/LICENSE
+    - library_licenses/urfave/cli/v2/LICENSE
+    - library_licenses/yaml.v2/LICENSE
+    - library_licenses/yaml.v2/NOTICE


Can this just be - library_licenses?

Sure. Updated.

isuruf · 2022-03-30T16:40:13Z

You called yours go-s5cmd. Is that agreed upon now? I know we're doing it for R. You had a couple more flags for building, should they be included?

It's not necessary to call it go-s5cmd because go packages build executables and shared libraries and the packaged parts do not have go code in it.

BastianZim · 2022-03-31T16:19:58Z

@isuruf Happy for me to merge? And thanks for the review.

benjaminrwilson · 2022-04-02T14:19:19Z

Thanks, @BastianZim!

Add s5cmd.

9e8025f

benjaminrwilson marked this pull request as ready for review March 12, 2022 04:58

benjaminrwilson added 3 commits March 12, 2022 00:03

Update recipe.

387310d

Update recipe.

43c6df3

Update recipe.

08e0a4d

xhochy reviewed Mar 12, 2022

View reviewed changes

recipes/s5cmd/meta.yaml Outdated Show resolved Hide resolved

benjaminrwilson added 5 commits March 12, 2022 13:21

Add lib licenses.

5fbc515

Update license.

8e34bda

Remove .go files.

d980a1a

Remove additional files.

b353b59

Remove yml.

ff8efda

BastianZim mentioned this pull request Mar 15, 2022

add s5cmd #18339

Closed

9 tasks

BastianZim reviewed Mar 21, 2022

View reviewed changes

Update meta.yaml

191568c

BastianZim reviewed Mar 21, 2022

View reviewed changes

recipes/s5cmd/meta.yaml Outdated Show resolved Hide resolved

Add licenses.

a002aae

BastianZim approved these changes Mar 21, 2022

View reviewed changes

BastianZim requested a review from xhochy March 21, 2022 18:19

BastianZim requested a review from ocefpaf March 25, 2022 14:41

BastianZim requested a review from isuruf March 30, 2022 15:54

isuruf reviewed Mar 30, 2022

View reviewed changes

benjaminrwilson added 2 commits March 30, 2022 13:17

Update meta.yaml

476ef43

Update meta.yaml

b01199c

BastianZim merged commit ac59c5a into conda-forge:main Apr 1, 2022

benjaminrwilson deleted the add-s5cmd branch April 2, 2022 14:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add s5cmd. #18306

Add s5cmd. #18306

benjaminrwilson commented Mar 12, 2022 •

edited

Loading

conda-forge-linter commented Mar 12, 2022

benjaminrwilson commented Mar 12, 2022

benjaminrwilson commented Mar 14, 2022

benjaminrwilson commented Mar 16, 2022

benjaminrwilson commented Mar 17, 2022

benjaminrwilson commented Mar 21, 2022

BastianZim left a comment

BastianZim Mar 21, 2022

benjaminrwilson Mar 21, 2022

BastianZim Mar 21, 2022

benjaminrwilson Mar 21, 2022

BastianZim Mar 21, 2022

ocefpaf Mar 22, 2022

BastianZim commented Mar 21, 2022

BastianZim commented Mar 21, 2022

BastianZim commented Mar 24, 2022

BastianZim commented Mar 24, 2022

ocefpaf commented Mar 26, 2022

isuruf Mar 30, 2022

benjaminrwilson Mar 30, 2022

isuruf commented Mar 30, 2022

BastianZim commented Mar 31, 2022

benjaminrwilson commented Apr 2, 2022

Add s5cmd. #18306

Add s5cmd. #18306

Conversation

benjaminrwilson commented Mar 12, 2022 • edited Loading

conda-forge-linter commented Mar 12, 2022

benjaminrwilson commented Mar 12, 2022

benjaminrwilson commented Mar 14, 2022

benjaminrwilson commented Mar 16, 2022

benjaminrwilson commented Mar 17, 2022

benjaminrwilson commented Mar 21, 2022

BastianZim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BastianZim commented Mar 21, 2022

BastianZim commented Mar 21, 2022

BastianZim commented Mar 24, 2022

BastianZim commented Mar 24, 2022

ocefpaf commented Mar 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isuruf commented Mar 30, 2022

BastianZim commented Mar 31, 2022

benjaminrwilson commented Apr 2, 2022

benjaminrwilson commented Mar 12, 2022 •

edited

Loading