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

Formatting rules for codebase #956

Closed
enessoylu opened this issue Apr 25, 2018 · 7 comments
Closed

Formatting rules for codebase #956

enessoylu opened this issue Apr 25, 2018 · 7 comments
Labels
front-burner p2 Nice to have

Comments

@enessoylu
Copy link

Is there any plan to define code formatting rules? There is a great need of it IMHO. Integrating TSLint to the project would be great.

@compulim
Copy link
Contributor

compulim commented May 3, 2018

I am not a big fans of prettier. But we should do a better job here. Some points we discussed before:

  • 2-space indent 4-space indent
  • No trailing spaces
  • Single empty line at end of file
  • Bracelets are required for one-liner if statement
  • Use const/let as much as possible
  • Sorted import for better merging

There are more potentially controversary things that come up in my mind:

  • Refactoring into multiple files
  • Don't publish *.ts to NPM, but individual ES5/6 with source map (inlined or as *.js.map)
  • Don't put *.js in the same folder as *.ts
  • Ternary operator vs. simple if
  • Coalesce using || vs. simple if
  • Migrate from nightmare to puppeteer so we can run tests on Travis
    • Also do some cleanup, make it faster, etc
  • Bumping versions

Some investigation areas:

  • Move to other Observable from rxjs for smaller output

@billba
Copy link
Member

billba commented May 3, 2018

If necessary I will die on the 4-space indent hill.

I'd love to be involved in the Observable conversation. I've been following the various libraries closely.

@compulim
Copy link
Contributor

compulim commented May 3, 2018

Okay, respect! Will keep that.

Bill, I have some big ambitions and we should talk after free up. 😉

@eanders-ms
Copy link
Contributor

I am all about the 2-space indent these days.

@billba
Copy link
Member

billba commented May 6, 2018

@eanders-ms we can't be friends anymore.

@cdagli
Copy link

cdagli commented May 9, 2018

@billba @eanders-ms What are your plans on that?

@cwhitten cwhitten added backlog Out of scope for the current iteration but it will be evaluated in a future release. and removed 4.4 labels Apr 2, 2019
@corinagum corinagum removed this from P2 in 4.4 Prioritization Apr 2, 2019
@compulim
Copy link
Contributor

@corinagum do you think #1982 resolved this issue? If not, please write down what is left.

@corinagum corinagum removed backlog Out of scope for the current iteration but it will be evaluated in a future release. labels Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
front-burner p2 Nice to have
Projects
None yet
Development

No branches or pull requests

7 participants