Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

mycli 1.0.0 (new formula) #42149

Closed
wants to merge 2 commits into from
Closed

Conversation

amjith
Copy link
Contributor

@amjith amjith commented Jul 26, 2015

mycli is a python application. It is a command line application to interact with MySQL database.

This is not a python library, but it's an application that happens to be written in Python.

The best installation test for mycli is to invoke it with the --help option. Which exercises all the libraries used by the app, which will be a robust test to see if all the dependencies are installed correctly.

url "https://pypi.python.org/packages/source/m/mycli/mycli-1.0.0.tar.gz"
sha256 "4d9258440b3a569066dc3c74a29d787bf89b0914aace080e6baa7dbf7ddb5f40"

bottle do
Copy link
Member

Choose a reason for hiding this comment

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

Please don't pre-add bottle blocks to new formula.

@amjith
Copy link
Contributor Author

amjith commented Jul 27, 2015

Thank you for the feedback.

I've removed the bottle block and added a blank line between the last end and the start of the def install.

Let me know if you have any other feedback.

@amjith
Copy link
Contributor Author

amjith commented Jul 27, 2015

Is there more work that is needed on this PR?

@amjith
Copy link
Contributor Author

amjith commented Jul 28, 2015

Is there anything I can do to make this PR easy to merge or review?

Waiting on the merge to announce the app.

/cc @DomT4

@amjith
Copy link
Contributor Author

amjith commented Jul 29, 2015

@tdsmith Would you mind reviewing this PR and merging?

You were the one shepherded the pgcli formula and this is very a similar formula.

Thank you!

end

test do
system bin/"mycli", "--help"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can’t we use something more elaborated than --help?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this app is an interactive client to a database, it is not possible to do a more elaborate test.

Doing an elaborate test will require knowing about a database instance and having access to a specific database.

But doing the --help should exercise the app and it's dependencies that are installed on the system.

@DomT4
Copy link
Member

DomT4 commented Jul 30, 2015

Will take this, but please note pinging maintainers for review/merge so soon after the PR is created is frowned upon - new formulae PRs are allowed to sit for a few days so that any maintainer that wants to look over it has the time to do so.

Merged in d1cdc52. Thank you for your contribution to Homebrew, we appreciate it!

@DomT4 DomT4 closed this in d1cdc52 Jul 30, 2015
@amjith
Copy link
Contributor Author

amjith commented Jul 30, 2015

Thank you for accepting the formula.

I would have preferred to know what was going on or how long it would take to come to a decision.

@amjith amjith deleted the clean_upstream branch July 30, 2015 06:57
@tdsmith
Copy link
Contributor

tdsmith commented Jul 30, 2015

Feel free to ping a ticket after a week or so. Thanks for submitting!

@Homebrew Homebrew locked and limited conversation to collaborators Jul 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants