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: give a hint to run init and deploy command #720

Conversation

adarshjhaa100
Copy link
Contributor

@adarshjhaa100 adarshjhaa100 commented Sep 25, 2023

What does this PR do?

The existing appwrite deploy [subcommand] gives a cryptic error message in absence of appwrite.json file in the working directory.
✗ Error No functions found in the current directory.

The feature has fixed this issue and now displays the following message instead:

✗ Error: No appwrite.json file found in the current directory. This command must be run in the folder holding your appwrite.json file. Please run this command again in the folder containing your appwrite.json file, or run [command to init appwrite.json].

The changes have been made for all of the services where where the cryptic message was displayed: function, collection, bucket and team

Fixes appwrite/sdk-for-cli#87

Test Plan

Testing Type : Manual

Scenario: When appwrite.json is not present in the working directory

image

Before :

image

After:

image
image
image
image

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

Yes

Copy link
Contributor

@stnguyen90 stnguyen90 left a comment

Choose a reason for hiding this comment

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

Great PR! 🤯 We left some comments during the review, please check them out.

@adarshjhaa100
Copy link
Contributor Author

adarshjhaa100 commented Oct 6, 2023

@stnguyen90 i've made the changes proposed in the review. See the last 2 commits.

All error messages now show up and have removed the unnecessary new property that I introduced for file check. config.data seems sufficient for it.

Some checks seem to be failing for other frameworks/languages like android, node etc. don't know why.

Copy link
Contributor

@stnguyen90 stnguyen90 left a comment

Choose a reason for hiding this comment

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

Some minor cleanup. Otherwise, I think this is good!

@adarshjhaa100
Copy link
Contributor Author

@stnguyen90 Thanks for pointing out. Removed unnecessary spaces and console log statements in the latest commit.

Copy link
Contributor

@stnguyen90 stnguyen90 left a comment

Choose a reason for hiding this comment

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

Hey 👋 awesome work on your PR! We've approved your work and it'll be merged soon!

@lohanidamodar
Copy link
Member

@adarshjhaa100 Great work on the PR. If you are still available will you be able to sync your PR with latest changes. If you are not working on this anymore, no worries, just let us know. Thank you.

@adarshjhaa100
Copy link
Contributor Author

@lohanidamodar sure, I'll sync my PR with the latest changes within this week.

@adarshjhaa100
Copy link
Contributor Author

@lohanidamodar I've synced and tested the latest changes. Do let me know in case of any issues.

@stnguyen90 stnguyen90 requested a review from lohanidamodar April 4, 2024 19:11
@lohanidamodar lohanidamodar merged commit 7a10919 into appwrite:master May 16, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 Feature: Give a hint to where you should run init and deploy commands
3 participants