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

[com.google.fonts/check/article/images] should pass if there is not an article/images dir #4703

Closed
vv-monsalve opened this issue May 9, 2024 · 5 comments
Assignees
Labels
Check improvement proposal GF's priority list List of high priority issues for google/fonts CI
Milestone

Comments

@vv-monsalve
Copy link
Collaborator

vv-monsalve commented May 9, 2024

Observed behavior

⚠️ WARN
There are 3 image files in the article directory and they should be moved to an article/images subdirectory:

The above Warn was reported on this CI run. However, in this comment on the issue proposal for a new check, Nathan Williams said the images subdirectory is not required.

Expected behavior

The check should pass if there is no images subdirectory under the article one (not reporting a Warn).

Resources and steps needed to reproduce

See CI run linked above

@vv-monsalve vv-monsalve added Check improvement proposal GF's priority list List of high priority issues for google/fonts CI labels May 9, 2024
@felipesanches
Copy link
Collaborator

It is just a WARN... for the sake of moving the repo towards some more homogeneous structure.

  • How many outliers do we actually have?
  • How hard would it be to reorganize the entire repo at once, moving all images to their respective images sub-dirs and afterwards making it mandatory?

I'd say there's some benefit for tooling and for user-friendliness to have a more regular directory structure.

@felipesanches
Copy link
Collaborator

It is just a WARN... for the sake of moving the repo towards some more homogeneous structure.

* How many outliers do we actually have?

* How hard would it be to reorganize the entire repo at once, moving all images to their respective `images` sub-dirs and afterwards making it mandatory?

I'd say there's some benefit for tooling and for user-friendliness to have a more regular directory structure.

@nathan-williams, I'd be glad to hear your thoughts here as well. What do you think?

@vv-monsalve
Copy link
Collaborator Author

vv-monsalve commented May 10, 2024

  • How many outliers do we actually have?

We are not currently using an "images" folder, so having it would be kind of the outlier

How hard would it be to reorganize the entire repo at once

It would require changing the paths in multiple HTML article files, which would be time-consuming on something that is not a strong requirement or need.

@emmamarichal, @m4rc1e, @yanone wdyt?

@emmamarichal
Copy link
Contributor

@vv-monsalve I agree. I've never seen a sub-dir called "images". What would that include, having another sub-folder for GIFS? videos?
I think we'd be complicating the process and it would actually require too many changes for something that I think works very well.

@felipesanches
Copy link
Collaborator

ok, no worries. I'll make the proposed change, then.

@felipesanches felipesanches self-assigned this May 15, 2024
@felipesanches felipesanches added this to the 0.12.7 milestone May 15, 2024
felipesanches added a commit to felipesanches/fontbakery that referenced this issue May 31, 2024
com.google.fonts/check/article/images
On the Google Fonts profile.

(issue fonttools#4703)
felipesanches added a commit to felipesanches/fontbakery that referenced this issue May 31, 2024
com.google.fonts/check/article/images
On the Google Fonts profile.

(issue fonttools#4703)
felipesanches added a commit that referenced this issue May 31, 2024
com.google.fonts/check/article/images
On the Google Fonts profile.

(issue #4703)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Check improvement proposal GF's priority list List of high priority issues for google/fonts CI
Projects
None yet
Development

No branches or pull requests

3 participants