search

gammapy / gamma-sky

Portal to the gamma-ray sky
http://gamma-sky.net/
MIT License
6 stars 3 forks source link

Change app to Angular 2 #29

Closed vorugantia closed 8 years ago

vorugantia commented 8 years ago

Should be alright this time. Sorry it took a while to make this commit

cdeil commented 8 years ago

Thanks!

There's a bunch of files in this pull request, e.g. gammasky/__init__.py, where every line is part of the diff, but there's no visible difference of what the diff is.

My guess is that the difference is line ending characters.

We have to understand and fix this, or we'll keep changing the line endings characters back and forth all the time as you and I edit files, and the diff will always be hard to review.

I found this link: https://help.github.com/articles/dealing-with-line-endings/ @vorugantia - Could you please read this (or other resources you find with Google) and find a way to fix this?

The goal is that you configure git (and maybe your text editor) to not introduce Windows line ending characters into the git repo. In this pull request, the gammasky/__init__.py file should not appear in the diff as changed.

After figuring it out, please also add a section at the end of https://github.com/gammapy/gamma-sky/wiki/Developer-documentation explaining that this project uses LF line endings and what Windows devs have to do to work with that?

cdeil commented 8 years ago

There's two test fails on travis-ci: https://travis-ci.org/gammapy/gamma-sky/builds/145332327

@vorugantia - Can you reproduce them locally? Do you understand them and know how to fix them? If no, I can have a look. Let me know.

cdeil commented 8 years ago

@vorugantia - I've done a code review and left some inline comments. The most important point is to find a good way to deal with the line endings issue, so that this will not come up over and over as we both work on the code.

There's other things, like the code dealing with catalogs should be split out into separate files, the popups should maybe be components. We could just continue with refactoring and improving the app in this PR, or merge and continue in other PRs. Either way will work. @vorugantia - Up to you.

vorugantia commented 8 years ago

@cdeil - I've opened a new pull request with all of the revisions.