-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add section about directory/file naming to the Readme #625
Conversation
README.md
Outdated
|
||
- components: React native components that are re-used in several places. | ||
- libs: Library classes/functions, these are not React native components (ie: they are not UI) | ||
- pages: These are components that define pages in the app. The component that defines de page itself should be named |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the page itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hehe spanish creeping in here, changing...
|
||
When adding new API commands (and preferrably when starting using a new one that was not yet used in this codebase) always | ||
prefer to return the created/updated data in the command itself, instead of saving and reloading. ie: if we call `CreateTransaction`, | ||
we should prefer making `CreateTransaction` return the data it just created instead of calling `CreateTransaction` then `Get` rvl=transactionList |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer to return the created/updated data in the command itself
I'm confused by this statement. Are we talking about how to design the API command itself in PHP/Auth? e.g. if I add CreateTransaction
to this codebase then I should update the API service to return some data instead of fetching data again after it is created in this app?
If that's correct, I don't really have anything against this, but it might require a better example and explanation as to why we should do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's exactly what I meant, I will add some details to this to make it clearer why
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I re-read it and I am unsure how to make it clearer, can you suggest an edit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh it's cool I think this will just intuitively make more sense once we have more API commands conforming to this rule. at the moment, I couldn't think of anything that even did this so thought it was strange to mention here.
ok, went ahead and renamed some of the dirs to match the doc. Don't be scared about the diff, it's mostly renames, but please review/merge soon so I don't get more conflicts. |
FUCK! A lot of conflicts, I am resolving them now, but please merge @marcaaron |
|
||
When adding new API commands (and preferrably when starting using a new one that was not yet used in this codebase) always | ||
prefer to return the created/updated data in the command itself, instead of saving and reloading. ie: if we call `CreateTransaction`, | ||
we should prefer making `CreateTransaction` return the data it just created instead of calling `CreateTransaction` then `Get` rvl=transactionList |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh it's cool I think this will just intuitively make more sense once we have more API commands conforming to this rule. at the moment, I couldn't think of anything that even did this so thought it was strange to mention here.
I noticed it was not clear what naming/directory conventions we should use for this new codebase, I added some choices around that, I am totally open to changing them as I don't really care which choices we do, only that we have a defined set of guidelines so that we don't have to think about what file/directory name we use for each new file we add.
For some things, I went with what we have, but for others I went with what made more sense to me and could be applied mostly mindlessly when writing code, please let me know if I got anything wrong on or if you just plain not agree.
I also know that some existing things will don't match what is written here, I'll go ahead and update them once we agree.
Tests
None
Screenshots
None