Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reduce dist files to only the additional generated data #1162

Merged
merged 6 commits into from
Jun 5, 2024

Conversation

foolip
Copy link
Collaborator

@foolip foolip commented May 30, 2024

The duplication of data across source and dist means that any changes to
the source require regenerating dist. This frequently slows down work as
this needs to be done in a checkout. Most commonly, an updated
description from a code review suggestion results in failing checks
because the dist file is out of sync.

The new approach is that a dist file will only contain compat_features
when they are sourced from BCD, and only contain status when it is
computed as opposed to manually maintained.

The built packages/web-features/index.json is unchanged.

@foolip

This comment was marked as outdated.

@foolip
Copy link
Collaborator Author

foolip commented Jun 3, 2024

A small sacrifice is that Baseline high dates are not checked in, and instead only computed in the build step.

This is what I thought would happen, but for whatever reason it's not like that. I haven't checked why, it's fine if we still have the Baseline high dates.

@ddbeck
Copy link
Collaborator

ddbeck commented Jun 4, 2024

@foolip I pushed a couple of changes to show what it would look like to rename the files so that the dists sort after the YAML files. If/when you rebase, this is the procedure I used to rename the dists:

# untrack old dist files without deleting them
$ git rm --cached features/*.dist.yml

# rename dist files (`brew install rename` if you need this or do something clever with your shell of choice)
$ rename --subst .dist.yml .yml.dist features/*.dist.yml

# track dist files
$ git add features/*.yml.dist

foolip and others added 3 commits June 4, 2024 18:05
The duplication of data across source and dist means that any changes to
the source require regenerating dist. This frequently slows down work as
this needs to be done in a checkout. Most commonly, an updated
description from a code review suggestion results in failing checks
because the dist file is out of sync.

The new approach is that a dist file will only contain `compat_features`
when they are sourced from BCD, and only contain `status` when it is
computed as opposed to manually maintained.

A small sacrifice is that Baseline high dates are not checked in, and
instead only computed in the build step.

The built packages/web-features/index.json is unchanged.
@foolip
Copy link
Collaborator Author

foolip commented Jun 4, 2024

@ddbeck I have rebased now!

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

I'm happy with this, provided you fix the prettier error. Feel free to merge immediately (and we can clean up the mess in other PRs). Or we could schedule a time to merge this (on Friday, maybe?) and I can do my best to reduce the number of open PRs between now and then.

@foolip
Copy link
Collaborator Author

foolip commented Jun 5, 2024

This is a very conflict-y PR so I'll get this merged ASAP and just rebase everything else as needed.

@foolip foolip merged commit c250320 into web-platform-dx:main Jun 5, 2024
3 checks passed
@foolip foolip deleted the minimal-dist branch June 5, 2024 13:34
@foolip
Copy link
Collaborator Author

foolip commented Jun 5, 2024

The built packages/web-features/index.json is unchanged.

I reconfirmed this is the case after landing by comparing the built index.json from commit f1ebf69 and c250320.

foolip added a commit to foolip/web-features that referenced this pull request Jun 12, 2024
This was accidentally removed in web-platform-dx#1162.

It's not clear how the accident happened, but this is the only source
*.yml file removed, all other file removals were *.dist.yml, confirmed
through `git show --stat c250320 --diff-filter=D`.
ddbeck pushed a commit that referenced this pull request Jun 13, 2024
This was accidentally removed in #1162.

It's not clear how the accident happened, but this is the only source
*.yml file removed, all other file removals were *.dist.yml, confirmed
through `git show --stat c250320 --diff-filter=D`.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants