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

Remove "free" from font family descriptions #4444

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davelab6
Copy link
Member

This is an urgent set of changes to family descriptions, requested by an internal stakeholder.

@RosaWagner this will require a large set of families to be added to_sandbox.txt

@davelab6 davelab6 requested a review from RosaWagner March 30, 2022 16:36
@davelab6
Copy link
Member Author

The git diff isn't super clean due to this touching a lot of files that haven't been touched in many many years, so I'm going to follow up with a 2nd commit on this branch with minor fixes to make diffing cleaner.

@davelab6
Copy link
Member Author

OK this is now ready for review by internal stakeholders, I'll comment (or merge) when ready

I've also just added a 2nd commit to add the relevant families to_sandbox.txt, which I found with

git diff-tree --no-commit-id --name-only -r  COMMIT

and then processed with

cut -d/ -f1,2

@davelab6
Copy link
Member Author

@RosaWagner do you think its good practice to add the to_sandbox.txt change to each PR itself? Maybe we can have a CI check for this, so that it always happens? :)

@RosaWagner
Copy link
Contributor

RosaWagner commented Mar 31, 2022

@davelab6 i don’t think it is a good idea, we generate the lists with a script once a week to avoid conflict with the eng team, and the idea is to have this task automated thanks to the script, manual changes to the lists is messing up the checking workflow. If we all change the lists at the same time, we lose track of stuff, better to wait for one push to be finished before adding new stuff.

If you add the PR to traffic Jam with the labels:

  • To sandbox
  • Description/Metadata/OFL

then you can generate the whole sandbox list with gftools gen-push-lists script.

edit: also having changes to sandbox in each PR, will definitely create merge conflicts when one PR gets merged. Really advising against it.

Update line breaks and make general copy fixes
@chrissimpkins chrissimpkins force-pushed the davelab6-remove-free-desc branch from 6de0361 to 825e5c4 Compare August 20, 2022 13:37
@chrissimpkins
Copy link
Collaborator

chrissimpkins commented Aug 20, 2022

Rebased on main and took Dave's branch changes on all DESCRIPTION files, main side on all to_sandbox.txt files. There are no diffs in this branch on to_sandbox.txt now. Should be ready for review.

cc @davelab6

@chrissimpkins
Copy link
Collaborator

OK this is now ready for review by internal stakeholders, I'll comment (or merge) when ready

@davelab6 Waiting on this OK from you to merge

@chrissimpkins
Copy link
Collaborator

@davelab6 Are we ready to merge these changes?

@RosaWagner RosaWagner marked this pull request as draft July 6, 2023 09:55
@davelab6 davelab6 removed the request for review from RosaWagner July 11, 2024 15:41
@davelab6 davelab6 self-assigned this Jul 11, 2024
@davelab6 davelab6 reopened this Jul 11, 2024
@davelab6 davelab6 added this to the 2024 Q3 milestone Jul 11, 2024
@davelab6 davelab6 modified the milestones: 2024 Q3, 2024 Q4 Oct 7, 2024
@davelab6 davelab6 added the P2 label Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants