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

Add CHANGELOG for 1.1 #174

Merged
merged 5 commits into from Apr 30, 2014
Merged

Add CHANGELOG for 1.1 #174

merged 5 commits into from Apr 30, 2014

Conversation

MaxGabriel
Copy link
Contributor

Re: #167

Cocoapods, who I stole this changelog format from, has people add their new additions to the Master section of the changelog as they make PRs. Technically, all these commits should still be under the Master section (since 1.1 isn't released), but I wanted to show the layout of the changelog.


[Complete diff](https://github.com/inkling/Subliminal/compare/1.0.1...master)

_Because of the significant period of time between the 1.0.1 and 1.1 releases, the 1.1 changelog is heavily abridged._
Copy link
Contributor

Choose a reason for hiding this comment

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

Good note

@wearhere
Copy link
Contributor

wearhere commented Apr 7, 2014

Looks good, thanks @MaxGabriel! Looks like you already addressed my feedback (wow). Let me know what you think about the feature subdivisions. Otherwise (to your point in the pull request description) we'll hold this until 1.1 is released.

@MaxGabriel
Copy link
Contributor Author

I think when I rebased I destroyed this comment:

I like your subdivision of the features section. I wish we could indent "New SLElement Subclasses" and "Other API additions" one level more so the top level sections were just "Notable Features" and "Notable Bug Fixes", but it looks like GitHub doesn’t support h7's and if we made "Notable Features" and "Notable Bug Fixes" h5's they might be too prominent, plus future releases will only have "Enhancements" and "Bug Fixes" sections.

We could make "New SLElement subclasses" and "Other API additions" bullet points like the other "Notable Features", and then sub-bullet their existing lists. What do you think?

(Can respond in a sec)

@wearhere
Copy link
Contributor

wearhere commented Apr 7, 2014

Oh also can you tag the commit message as "fixes" (or "closes) #167 @MaxGabriel? See here. Makes cleaning up issues easier.

@wearhere wearhere modified the milestone: 1.1 Apr 7, 2014
@MaxGabriel
Copy link
Contributor Author

I made two gists showing how it would look using:

  1. h5s for the Notable Features/Notable Bug Fixes sections
  2. Indenting New SLElement Subclasses and Other API additions under Notable Features

I'm not a fan of (1) because I think the gray text for the section header looks really good, and the h5 uses black text. Also, its weird that the h5 and h6 are the same size, so it doesn't really look like h6 is a subheading of h5. Making the Notable Features/Fixes sections h4s might alleviate this, but that exacerbates the problem of making them too prominent.

I like indenting more, but one problem is that if I don't bold the New SLElement and Other API additions sections, they blend in too much with the sibling bullet points. But when I bold them, they look almost like a bigger section header than the grayed out h6 header.

So pretty much I just like the grayed out headers, and they sort of lock you in because even regular bolded text is more prominent than them. But that's mostly just my opinion—I can do whatever you want really.

@MaxGabriel
Copy link
Contributor Author

Cool, tagged the commit with [closes 167].

@wearhere
Copy link
Contributor

wearhere commented Apr 7, 2014

Thanks for preparing the Gists @MaxGabriel--great discussion too. I agree that the gray text looks nice, but I think that using h5s and h6s is the best/least worst way to convey the relationship between the sections. Fwiw it looks like Cocoapods has recently changed to using h5s.

@MaxGabriel
Copy link
Contributor Author

Fair enough—I'll probably do that tonight

edit: should be good now.

@wearhere
Copy link
Contributor

wearhere commented Apr 8, 2014

Cool thanks @MaxGabriel, the format looks great now. This will be the last thing to go in before tagging 1.1. Right before I merge it I'll ask you to update it to add one last change--the PR that resolves #143.

##### Notable Features

* Subliminal no longer requires your password to run from the command-line. Instead, users should pre-authorize their computer to run Subliminal. See the [Continuous Integration FAQ](https://github.com/inkling/Subliminal/wiki/Continuous-Integration#faq) for details.
[Jeffrey Wear](https://github.com/wearhere) [#181](https://github.com/inkling/Subliminal/pull/181)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this entry should specifically call out -login_password and --live as being deprecated?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, good idea to mention that--though I think it might be even nicer to put that up at the top somehow? The most important things for developers to know are "what this release requires of them", both that they move away from -login_password/--live (soft requirement) and that they use Xcode 5.1 (hard requirement).

We should also mention that they can still use iOS 5.1 (see the README), but that this will be the last release to support doing so. (When I tag 1.1 I'll update that part of the README btw.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, at the top we should put

##### Updated Requirements

* Subliminal now requires Xcode 5.1. iOS 5.1 is still supported, though this is the last release
to support iOS 5.1. To test in the iOS 5.1 Simulator, users must run OS X 10.8 and manually
add the iOS 5.1 Simulator to Xcode as described [here](http://stackoverflow.com/a/22494536/495611).
* Subliminal no longer requires the user's password to run from the command-line. Users
should stop passing the `-login_password` and `--live` options to the `subliminal-test` script
and instead [pre-authorize their computers to run Instruments](https://github.com/inkling/Subliminal/wiki/Continuous-Integration#faq).

And then we can amend line 15 (#181's line item) to simply read "Subliminal no longer requires the user's password to run from the command line."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really good organization, good thinking


## 1.1

[Complete diff](https://github.com/inkling/Subliminal/compare/1.0.1...master)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should read "1.0.1...1.1" at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch; I had it using master so the link worked, but with 1.1 about to be merged I'll change it

@wearhere
Copy link
Contributor

Ok @MaxGabriel I've made my final comments on the Changelog (see the diff). If you can make those changes, we're pretty much set for 1.1 then.

* If you installed Subliminal **using Git submodules**, execute `git checkout <tag>` from
Subliminal's root directory and commit your change in your project.
* If you installed Subliminal **using Cocoapods**, update your Podfile (optional) and then
execute `pod update Subliminal` from your project directory.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cocoapods just added a new feature where you can run pod update POD_NAME to update just that pod; with just pod update it would update every pod that wasn't bounded by version constraints

(Other than that unchanged from what you wrote)

@wearhere
Copy link
Contributor

Terrific @MaxGabriel, thanks again.

wearhere added a commit that referenced this pull request Apr 30, 2014
@wearhere wearhere merged commit 397f74c into inkling:master Apr 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants