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

shards: new port, version 0.8.1-devel #2101

Merged
merged 1 commit into from Jun 30, 2018

Conversation

conradwt
Copy link
Contributor

Description

Shards is a dependency manager for the Crystal Programming Language.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 10.14 18A314h
Xcode 10.0 10L177m

Verification
  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

depends_lib port:crystal

checksums rmd160 dc60ef24d2e9e812138e042659c6f1599c68a312 \
sha256 d4a56101262ae9a9ce4352f534d532941dc2a2429e67fc263460d682eba088ff
Copy link
Contributor

Choose a reason for hiding this comment

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

Add the size parameter at the end.

Copy link
Contributor Author

@conradwt conradwt Jun 27, 2018

Choose a reason for hiding this comment

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

Yes, I can add the size. How is the size specified and can you explain the reasoning for it?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a new parameter that is not well documented yet. But it is being added to all ports as we move forward. Knowing the file size acts as a sanity check and also allows port to provide feedback while downloading such as telling the user how much data is left to download.

PortGroup github 1.0

name shards
version 0.8.1-devel
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove these two lines because the GitHub portgroup already sets them to these values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do and thanks for the clarification.


github.setup crystal-lang shards 0.8.1-devel v

epoch 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the epoch because it is only added if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do and thanks for the clarification.

long_description Shards is a dependency manager for the Crystal Programming Language.

homepage https://github.com/crystal-lang/shards
master_sites https://github.com/conradwt/shards/archive/v${github.version}
Copy link
Contributor

Choose a reason for hiding this comment

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

These two lines can also be removed.

platforms darwin
supported_archs noarch
license Apache-2
maintainers {@conradwt gmail.com:conradwt}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can openmaintainer be added here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I can add openmaintainer. Can you explain the reasoning for it?

Copy link
Contributor

@mf2k mf2k Jun 27, 2018

Choose a reason for hiding this comment

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

It is normal practice for ports maintained by non-committers to be openmaintainer so that committers can make minor fixes without waiting for approval. Most often this means we can increase the revision as needed when a dependency gets updated and it affects your port.

@mf2k
Copy link
Contributor

mf2k commented Jun 26, 2018

I'm not sure why all of my comments didn't show up the first time.

@macportsbot
Copy link

Travis Build #2773 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@conradwt
Copy link
Contributor Author

conradwt commented Jun 27, 2018

@mf2k I have updated the Portfile with the requested changes and thanks for taking the time to review it.

@macportsbot
Copy link

Travis Build #2775 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@macportsbot
Copy link

Travis Build #2776 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@conradwt conradwt changed the title New Port: SHards [WIP] New Port: SHards Jun 27, 2018
@chrstphrchvz
Copy link
Contributor

@conradwt You might need to check that your new commit was rebased properly. It looks as if any MacPorts' commits from the past day got squashed into your commit, so now your commit is trying to update a bunch of unrelated files.

@macportsbot
Copy link

Travis Build #2779 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@macportsbot
Copy link

Travis Build #2780 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@macportsbot
Copy link

Travis Build #2781 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@macportsbot
Copy link

Travis Build #2782 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@macportsbot
Copy link

Travis Build #2783 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@pmetzger
Copy link
Member

Your commit message format isn't per our standards. Try shards: new port, version 0.8.1 or some such.

@conradwt conradwt changed the title [WIP] New Port: SHards shards: new port, version 0.8.1-devel Jun 27, 2018
@macportsbot
Copy link

Travis Build #2792 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.


categories www
platforms darwin
supported_archs noarch
Copy link
Member

Choose a reason for hiding this comment

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

Is it truly the case that this port installs no architecture specific files at all?

supported_archs noarch
license Apache-2
maintainers {@conradwt gmail.com:conradwt} \
openmaintainer
Copy link
Member

Choose a reason for hiding this comment

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

Generally these should be on the same line. It makes grepping easier.


destroot.args PREFIX=${prefix}

test.run no
Copy link
Member

Choose a reason for hiding this comment

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

Are you disabling tests for a specific reason?

@pmetzger
Copy link
Member

Made a few more comments. Sorry this is taking so long, @conradwt, we'll try to merge it expeditiously once it's all clean.

@conradwt conradwt force-pushed the new-port-shards branch 2 times, most recently from 875d840 to b7420f9 Compare June 28, 2018 21:48
@conradwt
Copy link
Contributor Author

@pmetzger I have updated the Portfile based on you most recent comments. Thanks again for the feedback.

@macportsbot
Copy link

Travis Build #2798 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@macportsbot
Copy link

Travis Build #2799 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@macportsbot
Copy link

Travis Build #2800 Errored.

Lint results
--->  Verifying Portfile for shards
--->  0 errors and 0 warnings found.

@pmetzger pmetzger merged commit b242c46 into macports:master Jun 30, 2018
@pmetzger
Copy link
Member

Merged! Sorry for all the delays, @conradwt!


github.setup crystal-lang shards 0.8.1-devel v

categories www
Copy link
Member

Choose a reason for hiding this comment

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

By the way, I only just realized, is this actually www specific? If not, maybe it belongs in another category?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This port is applicable to Crystal Programming Language and its web development framework called Amber. Next, this port would be in similar categories as the rb-bundler port. Thus, it would it maybe should be something like crystal, www, devel. Would this be better?

Copy link
Member

Choose a reason for hiding this comment

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

Probably, yes, though we don't yet have a crystal category. That said, if we get enough crystal code, one would be appropriate. Certainly devel would be a good addition. You should feel free to request that in a PR.

@mohd-akram
Copy link
Member

This seems like it should belong solely in devel as it is a package manager like devel/npm6. As far as I can tell, Amber is a web framework package that can be installed by shards, but it is not directly related to it.

@pmetzger
Copy link
Member

pmetzger commented Jul 2, 2018

Yah, it does make some sense for it to just be devel. @conradwt, please submit a pull request. If we later get enough crystal packages we can also have a crystal directory and category.

@conradwt
Copy link
Contributor Author

conradwt commented Jul 2, 2018

@pmetzger Where would one find the instructions for crafting a PR for moving a port from one category to another?

@pmetzger
Copy link
Member

pmetzger commented Jul 3, 2018

There's nothing to it. Just do a "git mv" on a branch, and also change the category line. It's that easy.

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