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.

Sign up for GitHub

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

Jump to bottom

todoman 1.8.0 (new formula) #9080

Closed
wants to merge 1 commit into from
Closed

todoman 1.8.0 (new formula) #9080

wants to merge 1 commit into from

Conversation

benjaminfrank
Copy link
Contributor

@benjaminfrank benjaminfrank commented Jan 19, 2017 •
edited
Loading

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

@benjaminfrank benjaminfrank mentioned this pull request Jan 19, 2017
Closed
@ilovezfs
Copy link
Contributor

Using the GitHub url:

iMac-TMP:~ joe$ brew audit --new-formula todoman
todoman:
  * GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)
Error: 1 problem in 1 formula
iMac-TMP:~ joe$

@WhyNotHugo
Copy link

@ilovezfs That's probably because todoman recently moved into GitHub when it became part of pimutils. Formerly, it was hosted at GitLab, as a personal project. Note that all former PRs and issues were also lost when moving into the org.

@ilovezfs
Copy link
Contributor

@hobarrera thanks for the additional info

ilovezfs
ilovezfs reviewed Jan 20, 2017
View reviewed changes
Formula/todoman.rb
depends_on :python3

resource "atomicwrites" do
url "https://pypi.python.org/packages/a1/e1/2d9bc76838e6e6667fde5814aa25d7feb93d6fa471bf6816daac2596e8b2/atomicwrites-1.1.5.tar.gz"
Copy link
Contributor

Choose a reason for hiding this comment

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

would be good to use files.pythonhosted.org for all of them

ilovezfs
ilovezfs reviewed Jan 20, 2017
View reviewed changes
Formula/todoman.rb
desc "Simple CalDAV-based todo manager"
homepage "https://todoman.readthedocs.io/"
url "https://pypi.python.org/packages/2f/06/a377b38dcd3cba5334f3e6444ab5a26a0d7b134a6d040ac46a16ed70568f/todoman-1.8.0.tar.gz"
sha256 "8ccd602da8b0870dab8273dd50631847b2b9ad7e902a8ec5bfe901e0ea6c72a2"
Copy link
Contributor

Choose a reason for hiding this comment

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

head "https://github.com/pimutils/todoman.git" otherwise brew livecheck will not work, unless we use the GitHub source archive tarball above, which would be another option in lieu of adding head.

Copy link

Choose a reason for hiding this comment

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

The GitHub tarballs don't work, because they lack git metadata.
The tarballs at PyPI have a python file which is generated when publishing, which contains the required metadata.

Copy link
Contributor

@ilovezfs ilovezfs Jan 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

OK, so we can use either the tag itself, or the pythonhosted URL and add head "https://github.com/pimutils/todoman.git"

Copy link

Choose a reason for hiding this comment

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

Yes, the cloned tag is okay (meaning "not the tarball for the tag" either).

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, we do :tag => "foo123", :revision => "abcdef..."

ilovezfs
ilovezfs reviewed Jan 20, 2017
View reviewed changes
Formula/todoman.rb

test do
ENV["LC_ALL"] = "en_US.UTF-8"
ENV["LANG"] = "en_US.UTF-8"
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really have to set both? Usually one or the other is sufficient depending on circumstances.

ilovezfs
ilovezfs reviewed Jan 20, 2017
View reviewed changes
Formula/todoman.rb
date_format = %Y-%m-%d
default_list = Personal
EOS
mkdir testpath/".calendar/Personal"
Copy link
Contributor

Choose a reason for hiding this comment

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

(testpath/".calendar/Personal").mkpath

ilovezfs
ilovezfs reviewed Jan 20, 2017
View reviewed changes
Formula/todoman.rb
EOS
mkdir testpath/".calendar/Personal"
system "#{bin}/todo", "new", "newtodo"
assert_match /newtodo/, shell_output("#{bin}/todo list", 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

assert_match "newtodo", shell_output("#{bin}/todo list")

ilovezfs
ilovezfs reviewed Jan 20, 2017
View reviewed changes
Formula/todoman.rb
end

def install
virtualenv_create(libexec, "python3")
Copy link
Contributor

Choose a reason for hiding this comment

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

this line is unnecessary. The depends_on :python3 takes care of it.

@benjaminfrank
Copy link
Contributor Author

Hope I got everything correct, especially the GH url with tag and head.

@ilovezfs ilovezfs changed the title Todoman 1.8.0 todoman 1.8.0 (new formula) Jan 21, 2017
@woodruffw woodruffw added the new formula PR adds a new formula to Homebrew/homebrew-core label Jan 22, 2017
@ilovezfs
Copy link
Contributor

@benjaminfrank thanks for the 🆕 formula!

@ilovezfs ilovezfs closed this in c69820d Jan 22, 2017
@benjaminfrank benjaminfrank deleted the todoman branch March 14, 2017 19:38
@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@WhyNotHugo WhyNotHugo WhyNotHugo left review comments

@ilovezfs ilovezfs ilovezfs approved these changes

Assignees
No one assigned
Labels
new formula PR adds a new formula to Homebrew/homebrew-core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@benjaminfrank @ilovezfs @WhyNotHugo @woodruffw