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

Release 0.23 - Thumbs Up #1484

Merged
merged 8 commits into from
Oct 7, 2016
Merged

Release 0.23 - Thumbs Up #1484

merged 8 commits into from
Oct 7, 2016

Conversation

shiftkey
Copy link
Member

@shiftkey shiftkey commented Oct 3, 2016 •
edited
Loading

Aiming to ship this Thursday 6 October AEST. Running through the test suite today/tomorrow.

@ryangribble
Copy link
Contributor

ryangribble commented Oct 3, 2016 •
edited
Loading

I'd like to include #1477 if upstream github API fix for create repository call gets resolved before the release ETA

Also I just ran into a minor overlooked thing caused by #1445 today which I've just pushed PR #1485 for, which should be included in the release

haacked
haacked reviewed Oct 3, 2016
View reviewed changes
Copy link
Contributor

@haacked haacked left a comment

Choose a reason for hiding this comment

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

Looking good!

ReleaseNotes.md

**Features**

- Added support for probing for an GitHub Enterprise instance - #1404 via @haacked
Copy link
Contributor

@haacked haacked Oct 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe this should say "Added support to test whether a URL points to a GitHub Enterprise instance"

ReleaseNotes.md
**Breaking Changes**

Repository identifiers returned from the GitHub API will exceed `Int32.MaxValue` in
around 12 months, based on current growth. We've decided to update everywhere we
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow. 😱

@shiftkey
Copy link
Member Author

shiftkey commented Oct 3, 2016

@ryangribble I've added those two PRs to my checklist

@shiftkey shiftkey force-pushed the release branch 2 times, most recently from cd6c199 to 9668d9f Compare October 4, 2016 10:10
@shiftkey
Copy link
Member Author

shiftkey commented Oct 4, 2016

I'll open up some tasks for those integration tests I've muted here tomorrow.

@ryangribble
Copy link
Contributor

ryangribble commented Oct 4, 2016 •
edited
Loading

Are you still shooting for Thursday release or going to go earlier?

Regarding #1477 I checked just now and the behaviour is unchanged. It's reasonably safe to 🚢 anyway since it just means that allowed merge method fields are ignored on repo creation, and the respective response fields are returned null in the response to the Create() call as well. Once github.com is rectified, our Create method and integration test should pass.

Meanwhile the Edit() and Get() repository methods work fine, so the workaround is to create a repo then use the Edit() method to set the allowed merge methods. I'd like to get support out for configuring those merge methods if we can, since it's a popular feature and it'd be good to support it this quickly after it was anounced. Thoughts?

@shiftkey
Copy link
Member Author

shiftkey commented Oct 4, 2016 •
edited by ryangribble
Loading

@ryangribble yeah, still shooting for Thursday. I'll look over #1477 in the morning and check the tests are good to go.

@ryangribble
Copy link
Contributor

If the Create() method is still no good, we could back out those changes if you aren't happy to release it in that state while waiting for github API fix. It will still be useful to release the Update() and Get() Repository methods since the functionality can totally be exercised with them

@shiftkey
Copy link
Member Author

shiftkey commented Oct 4, 2016

@ryangribble cool, I'll throw some feedback in that PR to see if we should partially ship it

@ryangribble
Copy link
Contributor

Turns out it's a doco error and the create method isn't going to support the feature at this time, so I'll fix up that PR tomorrow

@ryangribble
Copy link
Contributor

ryangribble commented Oct 4, 2016 •
edited
Loading

#1477 is updated and good to 🚢

@shiftkey
Copy link
Member Author

shiftkey commented Oct 7, 2016

Oops, missed this yesterday due to other commitments. I'll push this out this afternoon.

@shiftkey
Copy link
Member Author

shiftkey commented Oct 7, 2016

Published:

@shiftkey shiftkey merged commit 4bd3a44 into master Oct 7, 2016
@shiftkey shiftkey deleted the release branch October 7, 2016 03:48
@haacked
Copy link
Contributor

haacked commented Oct 7, 2016

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

@haacked haacked haacked left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@shiftkey @ryangribble @haacked