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

Report arbitrary release channel names #92

Merged
merged 3 commits into from Jun 25, 2018

Conversation

Projects
None yet
3 participants
@daviwil
Copy link
Member

daviwil commented Jun 25, 2018

Description of the Change

This change updates the release channel reporting logic to support arbitrary channel names by comparing the current Atom version against a regular expression. This will enable the new Atom Nightly release channel to be reported.

Alternate Designs

A simpler solution would just be to look for nightly in the Atom version just as we do for beta, but this solution gives us the flexibility to have other release channels in the future with less effort.

Benefits

We'll have usage metrics for other release channels!

Possible Drawbacks

None that I can think of.

Applicable Issues

atom/atom#17538

@daviwil daviwil requested review from jasonrudolph and annthurium Jun 25, 2018

expect(url).toContain('aiid=sushi')
})

it('reports an unknown release channel', async () => {

This comment has been minimized.

@daviwil

daviwil Jun 25, 2018

Author Member

'unknown' may not be the best name for this because it should indicate an Atom version that doesn't conform to the expected pattern. Any thoughts?

This comment has been minimized.

@jasonrudolph

jasonrudolph Jun 25, 2018

Member

Maybe "nonstandard" or "unrecognized"?

This comment has been minimized.

@daviwil

daviwil Jun 25, 2018

Author Member

👍 'unrecognized'

@daviwil daviwil referenced this pull request Jun 25, 2018

Merged

Implement RFC 002: Atom Nightly Releases #17538

12 of 12 tasks complete
expect(url).toContain('aiid=sushi')
})

it('reports an unknown release channel', async () => {

This comment has been minimized.

@jasonrudolph

jasonrudolph Jun 25, 2018

Member

Maybe "nonstandard" or "unrecognized"?


let url = Reporter.request.mostRecentCall.args[0]
expect(url).toContain('aiid=sushi')
})

This comment has been minimized.

@jasonrudolph

jasonrudolph Jun 25, 2018

Member

@daviwil: Since we know for sure that we want to support a nightly release channel, how would feel about adding a test specifically for the nightly release channel?

This comment has been minimized.

@daviwil

daviwil Jun 25, 2018

Author Member

Sure thing!

@daviwil
Copy link
Member Author

daviwil left a comment

Thanks @jasonrudolph! Made those changes

expect(url).toContain('aiid=sushi')
})

it('reports an unknown release channel', async () => {

This comment has been minimized.

@daviwil

daviwil Jun 25, 2018

Author Member

👍 'unrecognized'


let url = Reporter.request.mostRecentCall.args[0]
expect(url).toContain('aiid=sushi')
})

This comment has been minimized.

@daviwil

daviwil Jun 25, 2018

Author Member

Sure thing!

@annthurium
Copy link
Contributor

annthurium left a comment

looks good!

@jasonrudolph

This comment has been minimized.

Copy link
Member

jasonrudolph commented Jun 25, 2018

🚀

@daviwil

This comment has been minimized.

Copy link
Member Author

daviwil commented Jun 25, 2018

Thanks all! Think we'll have another release of metrics package within the next couple of weeks or should I go ahead and do a patch bump?

@annthurium

This comment has been minimized.

Copy link
Contributor

annthurium commented Jun 25, 2018

I was planning on doing a release of the metrics package after I merge #91. That's a riskier change though, so it might make sense to do a patch bump with just your changes. Up to you, though.

@daviwil

This comment has been minimized.

Copy link
Member Author

daviwil commented Jun 25, 2018

Cool! I'll go ahead and do a patch bump in the meantime. Thanks a bunch!

@daviwil daviwil merged commit 18d726d into master Jun 25, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@daviwil daviwil deleted the dw-release-channels branch Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.