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

chore: refactor and export scope terms #882

Merged
merged 19 commits into from
Jun 24, 2020
Merged

Conversation

marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented May 29, 2020

This change (choose one):

  • Breaks existing normative behavior (please add label "breaking")
  • Adds new normative requirements
  • Adds new normative recommendations or optional items
  • Makes only editorial changes (only changes informative sections, or
    changes normative sections without changing behavior)
  • Is a "chore" (metadata, formatting, fixing warnings, etc).

Commit message:

Export "manifest/within scope" and "URL/within scope".


Preview | Diff

@marcoscaceres marcoscaceres requested a review from mgiuca May 29, 2020 04:52
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@mgiuca
Copy link
Collaborator

mgiuca commented May 29, 2020

(FYI, I'm signing off for the weekend. We'll continue the reviews on Monday! Have a good weekend.)

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@marcoscaceres marcoscaceres requested a review from mgiuca June 3, 2020 23:20
@marcoscaceres
Copy link
Member Author

marcoscaceres commented Jun 3, 2020

@mgiuca looks like we were super close on this one and #880. Let me know how you'd like to proceed.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@marcoscaceres
Copy link
Member Author

@mgiuca, I think I've addressed all your feedback... hopefully all good to go!

@marcoscaceres
Copy link
Member Author

@mgiuca, II'l really like to get this landed this week, as then we can land #880 ... or would you prefer #880 to land first?

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@marcoscaceres
Copy link
Member Author

Ok, hopefully all good now @mgiuca? 🤞🤞🥺🤞🤞

@mgiuca
Copy link
Collaborator

mgiuca commented Jun 24, 2020

Hrm ... pr-preview doesn't seem to have been updated. It's still stuck on 3ed7528. (Come to think of it ... that's very old, it's plausible that my previous round of reviews I was reviewing an older version than what you had presented..... that might explain why I thought you didn't do some of my suggestions. If so, I'm very sorry.)

@marcoscaceres
Copy link
Member Author

Oh man, that's my fault! argh, how embarrassing. Seems I didn't push.

Copy link
Collaborator

@mgiuca mgiuca left a comment

Choose a reason for hiding this comment

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

Great! I've dropped a bunch of comments that are mostly nits. Happy with how this ended up. Please take into account my comments, then merge.

I do suspect my previous review was based on an old version. :| sorry.

It can't be that you forgot to push. pr-preview hasn't updated since June 11.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
@marcoscaceres
Copy link
Member Author

Great! I've dropped a bunch of comments that are mostly nits. Happy with how this ended up.

Me too! Thanks for all your help. It was a long slog, but this ended up 100 times better!

@marcoscaceres marcoscaceres merged commit 59ecaa8 into gh-pages Jun 24, 2020
@marcoscaceres marcoscaceres deleted the export_within_scope branch June 24, 2020 08:10
@mgiuca
Copy link
Collaborator

mgiuca commented Jun 25, 2020

Great, thanks @marcoscaceres !

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.

4 participants