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

feat: add getAllViews getAllTypes dropAllTypes and dropAllViews functions #784

Merged
merged 12 commits into from
Feb 22, 2022

Conversation

Julien-R44
Copy link
Member

@Julien-R44 Julien-R44 commented Feb 10, 2022

Proposed changes

Add getAllViews getAllTypes dropAllTypes and dropAllViews functions.

Also added dropTypes and dropViews flags in db:wipe and migration:fresh commands, like suggested here : #764 (comment)

Added cross-env for running tests from package.json

I'm doing a PR for the documentation right now 👍

Types of changes

What types of changes does your code introduce?

Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

Further comments

For MSSQL and Oracle, I just threw an error in the new functions as a warning, I really don't know these DB so I leave that to someone else 😄

@thetutlage
Copy link
Member

Dude. You are a beast. The tests are failing coz of some typing errors. You can please look into them?

@Julien-R44
Copy link
Member Author

Thanks man! I love working on Adonis!
Concerning the type error, yes indeed it was there for a while, I tried a solution, the scopes typing works fine, but I'm absolutely not sure because it's some deep typing here 😄 ! Please check that I haven't done anything wrong with this

@Julien-R44
Copy link
Member Author

it finally works, CI is back ! 👍

@RomainLanz
Copy link
Member

I am not sure about the "dropAllViews" method is not implemented for xxx. Create a PR to add the feature.

I know it will add more work, but I believe it would be better to learn how to implement it in those DBMS and implement the feature right away.

@Julien-R44
Copy link
Member Author

Yeah, I took the liberty of doing this because I noticed that this was also the case for other features in Oracle and MSSQL :

'"getAllTables" method is not implemented for oracledb. Create a PR to add the feature'

'Support for advisory locks is not implemented for mssql. Create a PR to add the feature'

And nobody had claimed their implementation in more than a year and a half. it seems that nobody uses these DB 😄
Anyway, if you think it's really necessary to do it before merging the PR, I can try to look into it asap. just let me know 👍

@Julien-R44 Julien-R44 mentioned this pull request Feb 19, 2022
@thetutlage
Copy link
Member

Looks good to me! Let's get it in

@thetutlage thetutlage merged commit f7aa5b9 into adonisjs:develop Feb 22, 2022
@Julien-R44 Julien-R44 deleted the feat/drop-views-types branch February 22, 2022 12:34
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.

3 participants