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

eject 0.1.12 (new formula) #6893

Closed
wants to merge 7 commits into from
Closed

Conversation

edwardloveall
Copy link
Contributor

  • 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>)?

end

test do
system "#{bin}/eject"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm aware that this is a pretty weak test. Right now the tool only prints out usage instructions or requires a xib which would be a lot of lines to embed for a test. If you think that's reasonable or there's a better way, let me know and I'll change it.

Copy link
Member

Choose a reason for hiding this comment

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

What's the smallest possible xib that can be created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like I can create one that's 16 lines long, which doesn't seem that bad:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<document type="com.apple.InterfaceBuilder3.CocoaTouch.XIB" version="3.0" toolsVersion="11134" systemVersion="15F34" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" colorMatched="YES">
    <dependencies>
        <plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="11106"/>
        <capability name="documents saved in the Xcode 8 format" minToolsVersion="8.0"/>
    </dependencies>
    <objects>
        <placeholder placeholderIdentifier="IBFilesOwner" id="-1" userLabel="File's Owner"/>
        <placeholder placeholderIdentifier="IBFirstResponder" id="-2" customClass="UIResponder"/>
        <view contentMode="scaleToFill" id="iN0-l3-epB">
            <rect key="frame" x="0.0" y="0.0" width="375" height="667"/>
            <autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/>
            <color key="backgroundColor" red="1" green="1" blue="1" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
        </view>
    </objects>
</document>

When run through eject, it creates the following output:

// Create Views
let view = UIView()
view.frame = CGRect(x: 0, y: 0, width: 375, height: 667)
view.autoresizingMask = [.flexibleHeight, .flexibleWidth]
view.backgroundColor = UIColor(red: 1, green: 1, blue: 1, alpha: 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I created a better test. Let me know what you think!

@bfontaine bfontaine added the new formula PR adds a new formula to Homebrew/homebrew-core label Nov 14, 2016
xcodebuild
bin.install "build/Release/eject.app/Contents/MacOS/eject"
frameworks_path = "build/Release/eject.app/Contents/Frameworks"
mv(frameworks_path, frameworks)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can drop the parentheses here.

@bfontaine
Copy link
Contributor

The build failed as well as the audit:

Error: 2 problems in 1 formula
eject:
  * GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)
  * GitHub repository too new (<30 days old)

@edwardloveall
Copy link
Contributor Author

Oh, I see it's failing on the repository not being popular enough. I guess I'll do a tab instead. Sorry!

@MikeMcQuaid
Copy link
Member

@edwardloveall Leave this open, we might include it anyway.

@edwardloveall
Copy link
Contributor Author

Sure thing. I can also squash the commits if there's nothing else to change before it's merged. Let me know.

SWIFT

system "eject --file view.xib > view.swift"
File.read("view.swift") == swift
Copy link
Contributor

Choose a reason for hiding this comment

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

Use assert_equal swift, shell_output("#{bin}/eject --file view.xib") instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


test do
xib = File.open "view.xib", "a+" do |file|
file.write <<-XML
Copy link
Member

Choose a reason for hiding this comment

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

Use EOS.undent here and then you can indent the line below (and all others) two characters more than before file.write

XML
end

swift = <<-SWIFT
Copy link
Member

Choose a reason for hiding this comment

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

Similarly use EOS.undent 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.

Done

end

test do
xib = File.open "view.xib", "a+" do |file|
Copy link
Member

Choose a reason for hiding this comment

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

Don't need the xib assignment here and use (testpath/"view.xib").write instead of the block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

url "https://github.com/Raizlabs/Eject/archive/0.1.12.tar.gz"
sha256 "a4dae3d37f780d274f53ed25d9dc1a27d5245289f9b8cbaaf8be71bc9334de18"

depends_on :xcode => :build
Copy link
Member

Choose a reason for hiding this comment

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

Because this was failing on builds of yosemite
`/` is a method on pathname that merges two paths together. So something like (testpath/"view.xib") will create a path to a view.xib file in the testing directory.

Calling `write` on a pathname will write content to the file specified by the path.
@MikeMcQuaid
Copy link
Member

Thanks for your contribution to Homebrew! Without people like you submitting PRs we couldn't run this project. You rock!

orn688 pushed a commit to orn688/homebrew-core that referenced this pull request Nov 18, 2016
Closes Homebrew#6893.

Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
@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.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants