Skip to content

Remove deprecated code#1408

Closed
alyssajoyner wants to merge 16 commits intomainfrom
alyssa/remove-deprecated-code
Closed

Remove deprecated code#1408
alyssajoyner wants to merge 16 commits intomainfrom
alyssa/remove-deprecated-code

Conversation

@alyssajoyner
Copy link
Copy Markdown
Contributor

@alyssajoyner alyssajoyner commented Oct 21, 2025

This PR removes deprecated code, components and styles.

Code

  • toBeCalledTimes is deprecated; replaced with toHaveBeenCalledTimes
  • toBeCalledWith is deprecated; replaced with toHaveBeenCalledWith
  • lightTheme is deprecated; replaced with theme
  • error is deprecated; replaced with errors
  • MutableDataFrame is deprecated; replaced with DataFrame
  • getDataProvider is deprecated; replaced with getSupplementaryQueryRequest
  • condition is deprecated

Components

  • HorizontalGroup and VerticalGroup are deprecated; replaced with Stack

Styles

  • Remove gf-form styles

NOTE: Select component is deprecated and should be replaced with Combobox. This work is not included in this PR due to testing issues with Combobox.

@alyssajoyner alyssajoyner marked this pull request as ready for review October 21, 2025 21:21
@alyssajoyner alyssajoyner requested a review from a team as a code owner October 21, 2025 21:21
@alyssajoyner alyssajoyner moved this from Incoming to Needs Review in Partner Datasources Oct 21, 2025
@alyssajoyner alyssajoyner moved this from Needs Review to Incoming in Partner Datasources Oct 22, 2025
@alyssajoyner alyssajoyner marked this pull request as draft October 22, 2025 03:52
@adamyeats adamyeats moved this from Incoming to In Progress in Partner Datasources Oct 22, 2025
@alyssajoyner alyssajoyner moved this from In Progress to Needs Review in Partner Datasources Oct 23, 2025
@alyssajoyner alyssajoyner marked this pull request as ready for review October 23, 2025 18:20
Comment on lines +200 to +207
{/* <Switch
value={builderState.liveView}
onChange={onOptionChange('liveView')}
label={labels.liveView.label}
tooltip={labels.liveView.tooltip}
inline
/> */}
</div>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we remove this?

</InlineFormLabel>
<Input
width={200}
// width={200}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we remove this?

/>
</div>
<div className="gf-form">
<div>
Copy link
Copy Markdown
Collaborator

@bossinc bossinc Oct 28, 2025

Choose a reason for hiding this comment

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

are we able to remove the divs without styling in this file?

Comment thread src/data/CHDatasource.ts
Comment on lines +205 to +208
// REMOVE this whole method
// getDataProvider(type: SupplementaryQueryType, request: DataQueryRequest<CHQuery>): Observable<DataQueryResponse> | undefined { ... }

// ADD this instead:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

but we didnt remove a function called getDataProvider right?

Do we need to keep this comment even if we did?

Comment thread src/data/adHocFilter.ts
const key = escapeKey(f.key);
const value = escapeValueBasedOnOperator(f.value, f.operator);
const condition = i !== adHocFilters.length - 1 ? (f.condition ? f.condition : 'AND') : '';
const condition = i !== adHocFilters.length - 1 ? 'AND' : '';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Won't this always make adHocFileters AND so you will never be able to use OR

Comment thread src/data/logs.ts
values: field.values,
const effectiveName = oneLevelDetected && field.name === DEFAULT_LOGS_ALIAS ? 'logs' : field.name;

const logLevel = LogLevel[effectiveName as keyof typeof LogLevel] ?? LogLevel.unknown;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changing this to ?? changes the behavor. Is this what we want?

Comment thread src/labels.ts
validateSql: {
label: 'Validate SQL',
tooltip: 'Validate SQL in the editor.',
tooltip: 'Validate SQL in the editor',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I believe tool tips that are sentences have periods at the end.

@bossinc bossinc moved this from Needs Review to In Progress in Partner Datasources Dec 9, 2025
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Partner Datasources Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants