Skip to content
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

dbms-plugins resolvers and tests added #331

Merged
merged 1 commit into from
Sep 16, 2021

Conversation

jk05
Copy link

@jk05 jk05 commented Sep 8, 2021

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?

Added resolvers and tests for the following dbms-plugin operations:

  • add-sources
  • list-sources
  • remove-sources
  • install
  • list
  • uninstall

What is the current behavior?

No resolvers or tests for and dbms-plugins operations

What is the new behavior?

Can now list/add/remove dbms-plugins and sources.

Does this PR introduce a breaking change?

no

Other information:

@jk05 jk05 requested a review from nglgzz September 9, 2021 08:38
@jk05 jk05 force-pushed the web-api-update_dbms-plugins branch 5 times, most recently from f05d567 to 2404dc5 Compare September 16, 2021 08:26
export class DbmsPluginSource extends DbmsPluginSourceBase {}

@ObjectType()
export class DbmsPluginInstalled extends DbmsPluginSource implements IDbmsPluginInstalled {
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand they're pretty much the same, so it probably doesn't matter, but shouldn't this class extend DbmsPluginSourceBase?

Copy link
Author

Choose a reason for hiding this comment

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

let me take a look, I cant recall why I did it this way

}

@ArgsType()
export class InstallOrUninstallDbmsPluginArgs {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you only have the plugin name here and not all the args that are needed for installing/uninstalling plugins, PluginNameArgs might be a more accurate name instead?

Copy link
Author

Choose a reason for hiding this comment

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

makes sense

@jk05 jk05 force-pushed the web-api-update_dbms-plugins branch from 2404dc5 to ffefa85 Compare September 16, 2021 09:19
@jk05 jk05 requested a review from nglgzz September 16, 2021 09:20
@jk05 jk05 merged commit fe30edf into neo4j-devtools:master Sep 16, 2021
@jk05 jk05 deleted the web-api-update_dbms-plugins branch September 16, 2021 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants