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

Remove import packaged environments #653

Merged
merged 6 commits into from
Jul 6, 2022

Conversation

b-butler
Copy link
Member

Note; To merge after #651

Description

Removes import_packaged_environments from the configuration.

Motivation and Context

This is a move to make #649 simpler and our dependence on signac's config to be purely at the project level in Python.

Checklist:

@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #653 (26c7b56) into master (04ec120) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #653      +/-   ##
==========================================
- Coverage   79.14%   79.07%   -0.07%     
==========================================
  Files          33       33              
  Lines        3241     3231      -10     
  Branches      687      685       -2     
==========================================
- Hits         2565     2555      -10     
- Misses        535      536       +1     
+ Partials      141      140       -1     
Impacted Files Coverage Δ
flow/environment.py 68.85% <ø> (ø)
flow/util/config.py 85.00% <ø> (-15.00%) ⬇️
flow/__init__.py 100.00% <100.00%> (+15.00%) ⬆️
flow/environments/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04ec120...26c7b56. Read the comment docs.

Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

LGTM, once the upstream PR #651 is merged we'll just need to make sure conflicts with #652 are adequately resolved (they should be orthogonal but I suspect because of squash merging we'll have some minor issues, should be easy to resolve with a merge rather than a rebase).

@b-butler b-butler marked this pull request as ready for review June 23, 2022 18:15
@b-butler b-butler requested review from a team as code owners June 23, 2022 18:15
@b-butler b-butler requested review from cbkerr and Tobias-Dwyer and removed request for a team June 23, 2022 18:15
Copy link
Member

@cbkerr cbkerr left a comment

Choose a reason for hiding this comment

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

Looks good to me! I didn't even know that this existed

@b-butler b-butler merged commit 54d1ae7 into master Jul 6, 2022
@b-butler b-butler deleted the refactor/remove-import-packaged-environments branch July 6, 2022 14:28
b-butler added a commit that referenced this pull request Jul 6, 2022
* depr: Warn on use of environment_modules

* depr: Warn on use of import_packaged_environments

* refactor!: Remove import_packaged_environments from configuration

* doc: Update changelog
b-butler added a commit that referenced this pull request Jul 6, 2022
* depr: Warn on use of environment_modules

* depr: Warn on use of import_packaged_environments

* refactor!: Remove environment_modules

* style: Rename internal variable

* Remove import packaged environments (#653)

* depr: Warn on use of environment_modules

* depr: Warn on use of import_packaged_environments

* refactor!: Remove import_packaged_environments from configuration

* doc: Update changelog

* doc: Update changelog
# 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.

3 participants