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

Changes based on review for #4 #8

Merged
merged 15 commits into from Mar 8, 2019
Merged

Changes based on review for #4 #8

merged 15 commits into from Mar 8, 2019

Conversation

jathak
Copy link
Member

@jathak jathak commented Feb 12, 2019

Highlights:

  • The migrator now extends RecursiveStatementVisitor, eliminating
    the need for BaseVisitor
  • Migration state for each stylesheet is now stored in
    StylesheetMigration, which keeps track of member scope, namespaces,
    and patches.
  • The migrator now always runs on all dependencies, with the
    command-line options now only used to determine which migrations are
    written to disk. This simplifies the migrator logic.
  • Since dependencies are migrated at the time the import rule is
    migrated, the global members of the dependency are forwarded onto the
    StylesheetMigration, which allows the namespacing of implicit
    dependencies to avoid the complicated logic of examining the entire
    dependency graph each time.
  • The tests now use test_descriptor to avoid the need for mocks.

Highlights:
- The migrator now extends RecursiveStatementVisitor, eliminating
the need for BaseVisitor
- Migration state for each stylesheet is now stored in
StylesheetMigration, which keeps track of member scope, namespaces,
and patches.
- The migrator now always runs on all dependencies, with the
command-line options now only used to determine which migrations are
written to disk. This simplifies the migrator logic.
- Since dependencies are migrated at the time the import rule is
migrated, the global members of the dependency are forwarded onto the
StylesheetMigration, which allows the namespacing of implicit
dependencies to avoid the complicated logic of examining the entire
dependency graph each time.
- The tests now use test_descriptor to avoid the need for mocks.
@jathak jathak mentioned this pull request Feb 12, 2019
@jathak jathak requested a review from nex3 March 4, 2019 21:54
Copy link
Contributor

@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.

Thanks for iterating on this so much!

@jathak jathak merged commit 0d28291 into master Mar 8, 2019
@jathak jathak deleted the refactor branch March 8, 2019 04:22
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.

None yet

2 participants