Skip to content

Factor old conda locator out of conda service#15276

Merged
karrtikr merged 9 commits into
microsoft:mainfrom
karrtikr:cond
Feb 2, 2021
Merged

Factor old conda locator out of conda service#15276
karrtikr merged 9 commits into
microsoft:mainfrom
karrtikr:cond

Conversation

@karrtikr

@karrtikr karrtikr commented Jan 30, 2021

Copy link
Copy Markdown

Almost no new code is added, just moved a bunch of code to other class.

Some methods of conda service are not specific to discovery and used all over the extension. We cannot disable those even when in discovery experiment. Hence put the locator part into CondaLocatorService, which we can directly delete later once we remove the experiment.

I plan to put CondaLocatorService behind the experiment flag in a later PR, where I'll also implement methods of CondaService using conda.ts, our new conda code that was added.

@karrtikr karrtikr added the no-changelog No news entry required label Jan 30, 2021
@karrtikr karrtikr changed the title Separate deprecated conda service from the original Separate old conda locator from conda service Jan 30, 2021
@karrtikr karrtikr force-pushed the cond branch 2 times, most recently from da91fb4 to 7d3f93a Compare January 31, 2021 21:47
@karrtikr karrtikr changed the title Separate old conda locator from conda service Factor old conda locator out of conda service Feb 1, 2021
Comment on lines +80 to +96
/**
* Interface carries the properties which are not available via the discovery component interface.
*/
export interface ICondaService {
readonly condaEnvironmentsFile: string | undefined;
getCondaFile(): Promise<string>;
isCondaAvailable(): Promise<boolean>;
getCondaVersion(): Promise<SemVer | undefined>;
getCondaFileFromInterpreter(interpreterPath?: string, envName?: string): Promise<string | undefined>;
}

export const ICondaLocatorService = Symbol('ICondaLocatorService');
/**
* @deprecated Use the new discovery component when in experiment, use this otherwise.
*/
export interface ICondaLocatorService {
readonly condaEnvironmentsFile: string | undefined;
getCondaFile(): Promise<string>;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Check out these interfaces to see what was moved.

Comment on lines 83 to +88
export interface ICondaService {
readonly condaEnvironmentsFile: string | undefined;
getCondaFile(): Promise<string>;
isCondaAvailable(): Promise<boolean>;
getCondaVersion(): Promise<SemVer | undefined>;
getCondaFileFromInterpreter(interpreterPath?: string, envName?: string): Promise<string | undefined>;
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The 4 properties left here cannot be implemented using the discovery component API, hence we cannot get rid of all of CondaService.

@karrtikr karrtikr marked this pull request as ready for review February 1, 2021 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants