-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
More ergonomic profile name handling #6157
Conversation
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
@@ -3,31 +3,42 @@ | |||
from copy import deepcopy | |||
from dataclasses import dataclass, field | |||
from pathlib import Path | |||
from typing import Dict, Any, Optional, Mapping, Iterator, Iterable, Tuple, List, MutableSet, Type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI-- Most of these import changes are just sorting... Nothing major changed here but having properly sorted imports matters so I'm doing a little cleanup whilst here.
@@ -190,28 +201,52 @@ def _get_rendered_profile( | |||
|
|||
@classmethod | |||
def collect_parts(cls: Type["RuntimeConfig"], args: Any) -> Tuple[Project, Profile]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code in this function will be basically what the SB team will use to generate the profile and project, which can then be used with the from_parts
method to generate RuntimeConfig
s using any valid profile name.
The difference will be potentially overriding the profile name when it's collected like so:
profile = cls.collect_profile(args=args, profile_name="a_valid_profile_name")
@iknox-fa will this need to go into 1.3 as well then? Do we see any risk or breaking change with this going into a patch? (still catching up on this in Slack...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Yep-- apparently they support 1.2+ so we'll ned this change in both. Nevermind, I found the docs on how it works. :) |
(cherry picked from commit 77dfec7)
(cherry picked from commit 77dfec7)
resolves #6156
Description
Allows for overriding the profile name when creating new runtime configs.
N.B. Per SB team we need to backport this to 1.2.
Checklist
changie new
to create a changelog entry