Skip to content

provider index: removed import from + refactored a few parts#15570

Merged
tldahlgren merged 3 commits intospack:developfrom
alalazo:refactor/provider_index
Mar 25, 2020
Merged

provider index: removed import from + refactored a few parts#15570
tldahlgren merged 3 commits intospack:developfrom
alalazo:refactor/provider_index

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Mar 19, 2020

The refactoring is instrumental to a further subclassing of _IndexBase to implement user defined bindings of provider spec for #15443

@alalazo alalazo mentioned this pull request Mar 19, 2020
Closed
7 tasks
@adamjstewart
Copy link
Copy Markdown
Member

adamjstewart commented Mar 20, 2020

Why are we changing from from X import Y? Is there a style guide somewhere or a reason to avoid that syntax? Just want to be consistent.

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Mar 20, 2020

@adamjstewart There are two main reasons from my side:

  1. For large files it gives a sense to the reader that something was imported from another location without having to look into all the import statements
  2. It helps avoiding issues with circular imports

@tldahlgren tldahlgren self-requested a review March 24, 2020 17:49
tldahlgren
tldahlgren requested changes Mar 24, 2020
View reviewed changes
alalazo added 3 commits March 25, 2020 13:23
@alalazo
provider index: removed import from + refactored a few parts
e46fc66 
The refactoring is instrumental to a further subclassing
of _IndexBase to implement user defined bindings of
provider specs.
@alalazo
spec dag: add dependence on mock configuration
2154420
@alalazo
Improved docstrings and code according to review comments
a7b6285
@alalazo alalazo force-pushed the refactor/provider_index branch from d15bf9b to a7b6285 Compare March 25, 2020 13:36
alalazo
alalazo commented Mar 25, 2020
View reviewed changes
@tldahlgren tldahlgren self-requested a review March 25, 2020 16:34
tldahlgren
tldahlgren approved these changes Mar 25, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@tldahlgren tldahlgren 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 the changes. Confirmed the tests work fine for me locally.

@tldahlgren tldahlgren merged commit b42a96d into spack:develop Mar 25, 2020
@alalazo alalazo deleted the refactor/provider_index branch March 25, 2020 16:48
@tldahlgren
Copy link
Copy Markdown
Contributor

tldahlgren commented Mar 25, 2020

Thank you for your contribution.

@alalazo alalazo mentioned this pull request Mar 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tldahlgren tldahlgren tldahlgren approved these changes

Assignees

No one assigned

Labels

refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alalazo @adamjstewart @tldahlgren