Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

Conversation

@nex3
Copy link
Contributor

@nex3 nex3 commented Jan 18, 2020

As discussed with @abhiomkar, this adds an _index.scss file to each package so that each package can be loaded with a single @use rule. As a bonus, that @use rule will automatically use the package's name as its namespace.

Here's an example of the difference between an API using each separate files versus index files.

@nex3 nex3 changed the title Add index stylesheets to each package refactor: Add index stylesheets to each package Jan 18, 2020
nex3 added 2 commits January 24, 2020 14:44
The migrator uses import-only files to figure out what @use rules to
generate, so this will ensure that downstream users will automatically
load names from the new index files rather than the individual
variables/mixins/functions files.
Copy link
Collaborator

@abhiomkar abhiomkar left a comment

Choose a reason for hiding this comment

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

Sent first round of review comments.

  • Can you make a CL and export into PR (See http://go/mdc-web-g3sync)? You wouldn't have to do any additional changes on GitHub except exporting a CL. (Internally we've good test coverage including screenshot tests and global tests).
  • Can we also update actual _mixin, _variables files to use these index files, using index files greatly improves readability of our Sass files.

Thanks!

Copy link
Contributor Author

@nex3 nex3 left a comment

Choose a reason for hiding this comment

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

Can you make a CL and export into PR (See http://go/mdc-web-g3sync)? You wouldn't have to do any additional changes on GitHub except exporting a CL. (Internally we've good test coverage including screenshot tests and global tests).

I tried running mdc import_pr 5491 but I got an error:

ERROR: Cannot find last imported revision. Use --force if you really want to proceed with the migration
  CAUSED BY: Cannot find reference '6fb7ccb68afea5ad782e79948764ca16f1b40468'

Can we also update actual _mixin, _variables files to use these index files, using index files greatly improves readability of our Sass files.

My main goal is to unblock downstream migrations as soon as possible. I'll try to make time to circle back to this later on, but it may need to wait until @jathak is back in the office and the team has more bandwidth again.

@abhiomkar
Copy link
Collaborator

I tried running mdc import_pr 5491 but I got an error:

ERROR: Cannot find last imported revision. Use --force if you really want to proceed with the migration
  CAUSED BY: Cannot find reference '6fb7ccb68afea5ad782e79948764ca16f1b40468'

Please use --force option to force snapshot the PR.

My main goal is to unblock downstream migrations as soon as possible. I'll try to make time to circle back to this later on, but it may need to wait until @jathak is back in the office and the team has more bandwidth again.

SGTM. Thanks!

@jathak
Copy link
Contributor

jathak commented Jan 28, 2020

  • Can we also update actual _mixin, _variables files to use these index files, using index files greatly improves readability of our Sass files.

I don't think there's currently a simple way to do this automatically, but writing one shouldn't be too challenging (since it's essentially just merging the @use rules and changing namespaces), but since it's a change that's local to the file, it makes sense to do that separately, once the main index change is done.

nex3 added 3 commits January 28, 2020 14:56
This fixes a couple bugs: mdc-theme's "color-palette" variables
weren't being hidden, and nested packages' files weren't updated.
@nex3
Copy link
Contributor Author

nex3 commented Jan 30, 2020

Obsoleted by #5539

@nex3 nex3 closed this Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants