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

Suggestion: rename app.delete() #489

Closed
hden opened this issue Mar 26, 2019 · 7 comments
Closed

Suggestion: rename app.delete() #489

hden opened this issue Mar 26, 2019 · 7 comments
Assignees

Comments

@hden
Copy link

hden commented Mar 26, 2019

FWIW it's very confusing. Every developer running into this will have to wonder: does it cleanup the local background tasks or delete the production database entirely?

The documentation does not help either.

Renders this app unusable and frees the resources of all associated services.
https://firebase.google.com/docs/reference/admin/node/admin.app.App#delete

By app does it mean the app object I initiated locally or does it mean the production database (used to be called Firebase App)?

Please consider a common alternative, such as disconnect.

Related to #91 (comment)

@google-oss-bot
Copy link

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not seem to follow the issue template. Make sure you provide all the required information.

@hiranya911
Copy link
Contributor

Renaming this is not an option. All our SDKs support this method (even the client SDKs), and a rename would be a massive breaking change. But we can add more details to the existing documentation.

FirebaseApp.delete() only cleans up the local FirebaseApp instance. It doesn't clean up any back-end resources, the same way initializeApp() doesn't create/init any back-end resources

@hiranya911
Copy link
Contributor

Hi @egilmorez. Lets make this clear in the new documentation you're currently working on.

@akauppi
Copy link

akauppi commented Jul 22, 2020

@egilmorez there are some other places in the documentation that are vague in a similar fashion. If you come across any "instance" in the text, please check if it's explicit what that points to. I get the feel that for the authors these things have been over clear, and the right context is often assumed from the reader.

@hiranya911
Copy link
Contributor

I'm going to close this as I don't see any immediate action items for this repo. As for the naming change suggested, unfortunately the app.delete() terminology is pretty entrenched in the existing Firebase dev ecosystem, and difficult to root out. Even in the upcoming modularized API designs, we are planning to keep the same terminology. But we will make it a point to explain the semantics in the API docs.

@Joshfindit
Copy link

Would it fit the scope and design of firebase-admin if there was a disconnect() method that was simply an alias for delete()?

Personally, delete() reads as "delete my database", and conjures up the same hesitation as sudo rm -rf or DROP TABLE

@hiranya911
Copy link
Contributor

delete() is the terminology we have been using for years across all platforms we support (web, Android, IOS, Java, Python etc). If we are to introduce new terminology we should do it across all platforms, and currently there's no strong reasons to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants