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

Agree on code style and conventions for JavaScript #206

Closed
novalis opened this issue Jul 18, 2011 · 8 comments
Closed

Agree on code style and conventions for JavaScript #206

novalis opened this issue Jul 18, 2011 · 8 comments

Comments

@novalis
Copy link
Contributor

novalis commented Jul 18, 2011

We did this for Java code in #24 and I think we should do the same for JavaScript.

I like the code conventions that Crockford gives: http://javascript.crockford.com/code.html

Also, there are two patterns in the code that I'd like to discuss changing:

  • the entering/exiting {function name} console.log statements (which make the output very noisy)
  • catching exceptions at such a high level (which makes it harder to figure out what line caused the exception)
@novalis
Copy link
Contributor Author

novalis commented Jul 18, 2011

Discussed on the call.

All agreed:

  • We'll use the Crockford code style (but the other patterns, e.g., for class creation; we'll stick with the OpenLayers-style approach we're currently using)
  • We'll remove the enter/exit console.log statements
  • We'll start catching specific exceptions at the lowest level possible

--nicholasbs

@novalis
Copy link
Contributor Author

novalis commented Jul 18, 2011

I've begun documenting this and a few other code conventions which are already being used here: DevelopersGuide#Codestyle (thanks to Frank for writing up the additional notes).

--nicholasbs

@novalis
Copy link
Contributor Author

novalis commented Jul 18, 2011

(In [705]) Remove enter/exit function log statements (updates #206)

--nicholasbs

@novalis
Copy link
Contributor Author

novalis commented Jul 18, 2011

(In [707]) Remove console output (updates #206)

--nicholasbs

@novalis
Copy link
Contributor Author

novalis commented Jul 18, 2011

I believe we've decided that we were going to start making the style changes piecemeal to old code as we work with it, and start using the new style for new code. Correct me if I'm wrong here.

Do we plan on removing the higher level exceptions in one shot? That seems less risky than changing all of the style at once.

--rmarianski

@novalis
Copy link
Contributor Author

novalis commented Jul 18, 2011

Yes, that sounds right to me: New code should use the new conventions, and old code should be updated as it's worked on.

Re: removing the high-level catches, I'm +1 to doing it all at once. I don't think there's a lot of risk in doing that and it should be pretty quick.

Sound good?

--nicholasbs

@novalis
Copy link
Contributor Author

novalis commented Jul 18, 2011

(In [757]) Remove high-level try-catch blocks (and update associated code to new style conventions); add lots of missing semicolons; remove trailing commas; remove cruft (updates #206)

--nicholasbs

@novalis
Copy link
Contributor Author

novalis commented Jul 18, 2011

I've just gone through and made a number of updates. Specifically:

  • I removed most of the high-level try-catch blocks that were making debugging harder
  • I removed some of the other try-catch blocks that
  • I left any blocks that appeared to be targeted at (and dealt with) a specific bug
  • since in removing this code I needed to reformat the associated blocks, I figured I might as well apply our new agreed upon code style to them.
  • I added a bunch of missing semicolons (and a trailing comma)
  • I removed some general cruft

From my testing, I don't believe these changes reveal any new errors (e.g., exceptions that were previously being silently caught and discarded). If this turns out to be wrong, I think we should fix them on a case by case basis and, if we do need to silently discard exceptions, make sure to document why (there are already several cases of this in the code and I left all of them as-is).

I'm closing as fixed for now. Feel free to reopen if necessary.

--nicholasbs

@novalis novalis closed this as completed Jul 18, 2011
anttileppa pushed a commit to megasense-development/OpenTripPlanner that referenced this issue Jun 25, 2018
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

1 participant