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

Various fixes to ImageResource processing algorithms #811

Merged
merged 1 commit into from
Oct 14, 2019

Conversation

mgiuca
Copy link
Collaborator

@mgiuca mgiuca commented Oct 11, 2019

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:

Normative:

  • Removed passing the name of the member to the processing algorithm. It
    just directly takes the array of image resources. This fixes a
    mismatch between the callers (which were already passing the array)
    and the definition of the algorithm.
  • Removed definition of steps for processing "src". This was doing
    almost nothing; just inline the parsing of the URL.
  • Copy "platform" into the output array.

Formatting:

  • Fixed links to "sizes" definition.
  • Remove redundant list in processing algorithm.

Preview | Diff

Normative:

- Removed passing the name of the member to the processing algorithm. It
  just directly takes the array of image resources. This fixes a
  mismatch between the callers (which were already passing the array)
  and the definition of the algorithm.
- Removed definition of steps for processing "src". This was doing
  almost nothing; just inline the parsing of the URL.
- Copy "platform" into the output array.

Formatting:

- Fixed links to "sizes" definition.
- Remove redundant list in processing algorithm.
@mgiuca
Copy link
Collaborator Author

mgiuca commented Oct 11, 2019

@aarongustafson Please have a look at this.

I noticed so many issues while reviewing w3c/image-resource#6 which were copied from here, that I think we should fix them upstream first (since they won't be deleted from Manifest for some time).

If you wouldn't mind, can you copy the changes into w3c/image-resource#6 as well? (The commit description summarizes all the changes which you could make by hand.)

Copy link
Collaborator

@aarongustafson aarongustafson left a comment

Choose a reason for hiding this comment

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

LGTM. Given that most of this is going to be backed out once ImageResource is ready, I don’t think we need to get super particular about the naming of manifest URL/base URL vs. context, etc.

@mgiuca mgiuca merged commit c3e18c8 into w3c:gh-pages Oct 14, 2019
christianliebel pushed a commit to christianliebel/manifest that referenced this pull request May 27, 2020
Normative:

- Removed passing the name of the member to the processing algorithm. It
  just directly takes the array of image resources. This fixes a
  mismatch between the callers (which were already passing the array)
  and the definition of the algorithm.
- Removed definition of steps for processing "src". This was doing
  almost nothing; just inline the parsing of the URL.
- Copy "platform" into the output array.

Formatting:

- Fixed links to "sizes" definition.
- Remove redundant list in processing algorithm.
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.

2 participants