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

Implements RFC 7 #26

Merged
merged 2 commits into from
Sep 29, 2017
Merged

Implements RFC 7 #26

merged 2 commits into from
Sep 29, 2017

Conversation

ashfurrow
Copy link
Contributor

Fixes #7, based on this work from Orta. Possible addition: translate from shorthand to longhand labels (so a commit message like [MSG] Implements inbox would match the Messaging label. I'll take a look later today.

There's probably some code golfing to do, so don't be shy about pointing out alternative syntax etc!

@ashfurrow
Copy link
Contributor Author

ashfurrow commented Sep 28, 2017

Strange, wonder why Peril didn't complain about no one being assigned to the PR?

@ashfurrow
Copy link
Contributor Author

Ah, I see the failure in the logs. I'll try to take a look.

@ashfurrow
Copy link
Contributor Author

Fixed that in #27.

@jonallured
Copy link
Member

This is awesome - I'm into it for sure.

My only hesitation is taking a step back and looking at all-prs.ts and wondering where this file is going. I'm seeing some duplication and maybe overthinking it, but this approach of using straight functions seems like it boxes us in. I do not have a better idea, just something that occurred to me as I was mulling this over. Has anyone else noticed this?

@orta
Copy link
Contributor

orta commented Sep 29, 2017

There's a current limitation with peril that we can't require relative files, so we're stuck with a long file ATM

@jonallured
Copy link
Member

Cool, going to merge, thanks @ashfurrow!!

@jonallured jonallured merged commit 88d9fff into master Sep 29, 2017
@orta orta deleted the ashfurrow-rfc-7 branch September 30, 2017 22:58
@orta
Copy link
Contributor

orta commented Oct 4, 2017

@ashfurrow
Copy link
Contributor Author

AWWWWWW YEAH

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.

3 participants