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

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 Nov 14, 2018
@hiranya911 hiranya911 self-assigned this Nov 14, 2018
Copy link
Member

@hiranya911 hiranya911 left a comment

LGTM. I manually changed the base to dev branch. Please do the same for future PRs.

Loading

@hiranya911
Copy link
Member

@hiranya911 hiranya911 commented Nov 14, 2018

@chemidy can you also add a quick entry to the CHANGELOG file before I merge this?

Loading

@hiranya911 hiranya911 merged commit 9f9a563 into firebase:dev Nov 14, 2018
2 checks passed
Loading
@broady
Copy link

@broady broady commented Dec 8, 2018

nit, for future reference: in idiomatic Go, this should be called "APSAlert". Acronyms are uppercase.

Loading

@hiranya911
Copy link
Member

@hiranya911 hiranya911 commented Dec 8, 2018

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.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants