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

Some friendly editorial changes #64

Closed
wants to merge 3 commits into from
Closed

Conversation

halindrome
Copy link
Contributor

A couple of typos and suggested wording changes. Use at your discretion.

A couple of typos and suggested wording changes.  Use at your discretion.
@@ -348,7 +348,7 @@
<tr>
<td>Payment App</td>
<td>Digital Wallet, Payment Instrument </td>
<td>A piece of software to pay</td>
<td>A piece of software to manage payment methods</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

It's more than that really, no? It's either: a piece of software that does pay or returns back a payment credential to enable payment.

@zkoch
Copy link
Contributor

zkoch commented Mar 17, 2016

This mostly LGTM. I'm not sure about the change to the payment app definition (though in general, I agree it should probably be modified).

@@ -358,7 +358,7 @@
<tr>
<td>Payment Mediator</td>
<td>Digital Wallet, Payment Instrument</td>
<td>A piece of software that coordinates messages between Web apps and Payment apps</td>
<td>A piece of software that coordinates messages between sellers and Payment apps</td>
</tr>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't agree with this change. The messages are exchanged between the website and the payment app via the mediator.

@halindrome
Copy link
Contributor Author

I don't hate that. It is closer to what I think should happen, and is loose enough that many different implementation strategies could be used.

@msporny
Copy link
Member

msporny commented Mar 18, 2016

+1 to @adrianhopebailie's suggested change above.

@adrianhopebailie
Copy link
Collaborator

@halindrome Can you update your PR to reflect the discussion here?

Updated to reflect agreed upon wording.
@halindrome
Copy link
Contributor Author

I have updated @adrianhopebailie - however, GitHub indicates that this is an outdated diff, so perhaps it was overcome by events anyway.

@adrianhopebailie
Copy link
Collaborator

@halindrome LGTM - looks like it will merge fine unless I am missing something

halindrome added a commit to halindrome/browser-payment-api that referenced this pull request Mar 30, 2016
As per Nick's request, reapplying the agreed upon change from w3c#64.
@adrianba
Copy link
Contributor

adrianba commented Apr 1, 2016

Merged as e72f051

@adrianba adrianba closed this Apr 1, 2016
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.

None yet

5 participants