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

FR #173 : add subtitle for ApsAlert #191

Merged
merged 4 commits into from
Nov 14, 2018
Merged

FR #173 : add subtitle for ApsAlert #191

merged 4 commits into from
Nov 14, 2018

Conversation

chemidy
Copy link
Contributor

@chemidy chemidy commented Nov 14, 2018

I Added subtitle Based on APS doc

#173
@hiranya911 hiranya911 changed the base branch from master to dev November 14, 2018 21:44
@hiranya911 hiranya911 self-assigned this Nov 14, 2018
hiranya911
hiranya911 approved these changes Nov 14, 2018
View reviewed changes
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM. I manually changed the base to dev branch. Please do the same for future PRs.
@hiranya911
Copy link
Contributor

@chemidy can you also add a quick entry to the CHANGELOG file before I merge this?
@hiranya911 hiranya911 merged commit 9f9a563 into firebase:dev Nov 14, 2018
@broady
Copy link

broady commented Dec 8, 2018

nit, for future reference: in idiomatic Go, this should be called "APSAlert". Acronyms are uppercase.
@hiranya911
Copy link
Contributor

Thanks @broady. We considered that. It's not clear from the Apple documentation whether aps is an acronym. We are fairly certain it doesn't stand for anything. It's often just referred to as the "aps dictionary". I've only seen one place where it's spelled out as APS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants
@chemidy @hiranya911 @broady