Skip to content

Renamed 'interface managers' to something more specific#1510

Merged
charisk merged 7 commits intomainfrom
charisk/webview-improvements
Sep 14, 2022
Merged

Renamed 'interface managers' to something more specific#1510
charisk merged 7 commits intomainfrom
charisk/webview-improvements

Conversation

@charisk
Copy link
Copy Markdown
Contributor

@charisk charisk commented Sep 14, 2022

The term "interface manager" is used to represent the interface between the core extension run time and a certain web view. The name seems a bit too vague to me so I've made some renames:

  • AbstractInterfaceManager -> WebviewBase
  • Rename CompareInterfaceManager -> CompareView
  • InterfaceManager -> ResultsView
  • RemoteQueriesInterfaceManager -> RemoteQueriesView
  • VariantAnalysisInterfaceManager -> VariantAnalysisView

See individual commits for the exact renames.

@charisk charisk requested review from a team as code owners September 14, 2022 12:24
Copy link
Copy Markdown
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

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

LGTM, although I'd prefer the WebviewBase to be named something like AbstractView just so it's the same pattern as the views. This is just a personal preference though, I'm also happy to go with WebviewBase.

sandbox = sinon.createSandbox();

localQueriesInterfaceManagerStub = {
localQueriesResutsViewStub = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
localQueriesResutsViewStub = {
localQueriesResultsViewStub = {

Comment thread extensions/ql-vscode/src/interface.ts Outdated
}

export class InterfaceManager extends AbstractInterfaceManager<IntoResultsViewMsg, FromResultsViewMsg> {
export class ResultsView extends WebviewBase<IntoResultsViewMsg, FromResultsViewMsg> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this file be called view.ts rather than interface.ts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was tempted to rename it but it has a bunch of other stuff in there so it didn't make sense to rename it.

@charisk charisk added the secexp label Sep 14, 2022
@charisk charisk enabled auto-merge (squash) September 14, 2022 13:02
@charisk charisk merged commit 09dccc1 into main Sep 14, 2022
@charisk charisk deleted the charisk/webview-improvements branch September 14, 2022 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants