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

Code cleanup study #30

Closed
brauliodiez opened this issue Mar 17, 2016 · 5 comments
Closed

Code cleanup study #30

brauliodiez opened this issue Mar 17, 2016 · 5 comments
Assignees

Comments

@brauliodiez
Copy link
Member

Current sample code base needs a good cleanup, I propose @algil to create first a list of enhancements, then we can break / group them by issues and proceed with the cleanup task.

About the list to be generated, I would propose something like:

  • Enhancementes that must be applied:
    • Field name FireValidationFieldValueChange: wrong name it just updates ui it should be FireFieldValueChange
    • Remove imports that are not being used (e.g. memberapi in reducers).
    • (...) <-- @algil to create the whole list
  • Enhancements that needs discussion set an standard and then be implemented / set coding standard rule:
    • Imports, should we remove the "default" and use always {} ?
    • (...)

A later stage will be to configure tslinter

To complete this task you can answer in the task listing the enhancementes in a single comment.

@antoniogiroz
Copy link
Collaborator

Sounds good. I will work on it

@brauliodiez
Copy link
Member Author

Let's start giving here ideas, and then compile them:

Must change:

  • FireValidationFieldVallueChange to FireFieldValueChange
  • Remove unnceseary imports (e.g. memberapi on reducers).

To be discussed:

  • Avoid export default
  • Separate presentationales from container.
  • Create a enum / const like class for actions types..

@antoniogiroz
Copy link
Collaborator

I add more items to list.

Must change:

  • FireValidationFieldVallueChange to FireFieldValueChange
  • Remove unnecessary imports (e.g. memberapi on reducers).
  • Unify variable names in imports (e.g. Sometimes we write 'memberAPI' and in other cases we write 'MemberAPI'. In my opinion, in this case we should write in camelCase due to a new instance is created when it's exported. In other case should be in PascalCase.
  • Change tabs to 2 spaces characters for indentation and reformat code. For this, I recommend using tslint.json file.
  • Fixing syntax of properties in several interfaces. Change ',' for ';' and putting in the end of the property name.
  • Unify string quotes. Use single quotes ' or double quotes but not both. The typescript coding guidelines by Microsoft proposes to use doble for strings: https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines

To be discussed:

  • Avoid export default and import classes with curly-bracket {}
  • Separate presentationales from container.
  • Create a enum / const like class for actions types..

@brauliodiez
Copy link
Member Author

Lets start with the first two items in the list

Sent from my Windows Phone

-----Mensaje original-----
De: "Antonio L. Gil" notifications@github.com
Enviado: ‎19/‎03/‎2016 12:06
Para: "Lemoncode/react-typescript-samples" react-typescript-samples@noreply.github.com
CC: "Braulio Diez" braulio.diez@gmail.com
Asunto: Re: [react-typescript-samples] Code cleanup study (#30)

I add more items to list.
Must change:
FireValidationFieldVallueChange to FireFieldValueChange
Remove unnecessary imports (e.g. memberapi on reducers).
Unifying variable names in imports (e.g. Sometimes we write 'memberAPI' and in other cases we write 'MemberAPI'. In my opinion, in this case we should write in camelCase due to a new instance is created when it's exported. In other case should be in PascalCase.
Change tabs to 2 spaces characters for indentation and reformat code. For this, I recommend using tslint.json file.
Fixing syntax of properties in several interfaces. Change ',' for ';' and putting in the end of the property name.
To be discussed:
Avoid export default and import classes with curly-bracket {}
Separate presentationales from container.
Create a enum / const like class for actions types..

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@brauliodiez
Copy link
Member Author

Take as well the one about using same names fornthe imports.

This three tasks are heavy because we need to propagate them to all the samples plus test that we havent broken anthing.

On the other Hand it will enhance code readibility

Sent from my Windows Phone

-----Mensaje original-----
De: "braulio.diez@gmail.com" braulio.diez@gmail.com
Enviado: ‎19/‎03/‎2016 12:27
Para: "Lemoncode/react-typescript-samples" reply@reply.github.com; "Lemoncode/react-typescript-samples" react-typescript-samples@noreply.github.com
Asunto: RE: [react-typescript-samples] Code cleanup study (#30)

Lets start with the first two items in the list

Sent from my Windows Phone

De: Antonio L. Gil
Enviado: ‎19/‎03/‎2016 12:06
Para: Lemoncode/react-typescript-samples
CC: Braulio Diez
Asunto: Re: [react-typescript-samples] Code cleanup study (#30)

I add more items to list.
Must change:
FireValidationFieldVallueChange to FireFieldValueChange
Remove unnecessary imports (e.g. memberapi on reducers).
Unifying variable names in imports (e.g. Sometimes we write 'memberAPI' and in other cases we write 'MemberAPI'. In my opinion, in this case we should write in camelCase due to a new instance is created when it's exported. In other case should be in PascalCase.
Change tabs to 2 spaces characters for indentation and reformat code. For this, I recommend using tslint.json file.
Fixing syntax of properties in several interfaces. Change ',' for ';' and putting in the end of the property name.
To be discussed:
Avoid export default and import classes with curly-bracket {}
Separate presentationales from container.
Create a enum / const like class for actions types..

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@nasdan nasdan closed this as completed Mar 30, 2016
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

No branches or pull requests

3 participants