Skip to content
This repository was archived by the owner on Sep 29, 2023. It is now read-only.

Conversation

christyjacob4
Copy link
Contributor

@christyjacob4 christyjacob4 commented Feb 6, 2022

Screenshot 2022-02-07 at 3 15 18 AM

Copy link
Member

@eldadfux eldadfux left a comment

Choose a reason for hiding this comment

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

What should we do with the 1st table in the page? I feel the difference between them is not very clear for the reader.

@christyjacob4
Copy link
Contributor Author

@eldadfux the first table talks about the convention we follow when deciding an HTTP status code for our errors. And the second table talks more about Appwrite specific error codes.

@eldadfux
Copy link
Member

eldadfux commented Feb 8, 2022

I would only consider adding the version we started supporting each error code. We have something similar for env vars.

@christyjacob4
Copy link
Contributor Author

@eldadfux In this case, I'm not sure we can follow the convention we use for ENV Vars.
Screenshot 2022-02-08 at 7 41 46 PM

Since we introduced all these error codes in 0.13. literally every row in the table will have this information and it'll just make the table very noisy.

It's probably a better option to mention that this feature was introduced in 0.13 once in one of the paragraphs above.

From 0.14 onwards, we can start following the ENV Vars convention

@christyjacob4
Copy link
Contributor Author

christyjacob4 commented Feb 8, 2022

Screenshot 2022-02-08 at 8 46 53 PM

@eldadfux updated the description to add the note

Copy link
Contributor

@gewenyu99 gewenyu99 left a comment

Choose a reason for hiding this comment

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

LGTM <3

@christyjacob4
Copy link
Contributor Author

Latest Preview

Screenshot 2022-02-09 at 2 49 56 AM

Copy link
Contributor

@Meldiron Meldiron left a comment

Choose a reason for hiding this comment

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

LGTM

@eldadfux eldadfux changed the base branch from main to 0.13 February 14, 2022 06:22
@eldadfux
Copy link
Member

Changed target branch to 0.13

@eldadfux eldadfux merged commit 431bed8 into 0.13 Feb 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants