daxtens / finvis

Web-based financial data visualisation. Looking for developers!
GNU General Public License v3.0
7 stars 3 forks source link

Fix private/protected/public methods #2

Closed daxtens closed 11 years ago

daxtens commented 11 years ago

30 mins ($20)

viewobj.js has 2 functions with // todo: make me private. or something like that. Find them, and make them private/protected/whatever is appropriate (per Crockford's article)

There are some other methods that should obviously be moved: I think _render or something like that. Do the obvious ones in a single commit, if there are any you're not so sure about, do them in a separate commit, or just ask.

If you have time left over, repeat this process with other JS files.

axtens commented 11 years ago

//todo: make me private/protected/something

I don't know what you mean by this? Private to what? Protected by what? Sorry, I need clarification of what you're wanting to achieve.

daxtens commented 11 years ago

I think I explained that earlier - let me know if you're still stuck.

axtens commented 11 years ago

Finished the task ... I think ... and code committed

daxtens commented 11 years ago

You need to do a git push. http://www.kernel.org/pub/software/scm/git/docs/everyday.html#Individual%20Developer%20(Participant)

axtens commented 11 years ago

That didn't help me much. How about you just say "enter the following git command" and show me what it is

axtens commented 11 years ago

Hang on. Might have figured it out.

axtens commented 11 years ago

No. Going round in circles again.

C:\Users\BruceAxtens\Resources\Clients\Daniel\finvis>git push Username for 'github.com': Password for 'github.com': To https://github.com/daxtens/finvis.git ! [rejected] master -> master (non-fast-forward) error: failed to push some refs to 'https://github.com/daxtens/finvis.git' To prevent you from losing history, non-fast-forward updates were rejected Merge the remote changes (e.g. 'git pull') before pushing again. See the 'Note about fast-forwards' section of 'git push --help' for details.

C:\Users\BruceAxtens\Resources\Clients\Daniel\finvis>git push --help Launching default browser to display HTML ...

C:\Users\BruceAxtens\Resources\Clients\Daniel\finvis>cd ..

C:\Users\BruceAxtens\Resources\Clients\Daniel>start .

C:\Users\BruceAxtens\Resources\Clients\Daniel>git push https://github.com/daxtens/finvis.git fatal: Not a git repository (or any of the parent directories): .git

C:\Users\BruceAxtens\Resources\Clients\Daniel>

If I do a pull, does that overwrite my changes?

daxtens commented 11 years ago

No, a git pull won't overwrite, it will do a merge.

axtens commented 11 years ago

Okay. I pulled, then pushed and something got pushed

axtens commented 11 years ago

By the way, which Python are you using? The versions available to me are at http://www.activestate.com/activepython/downloads

On Fri, Jan 25, 2013 at 12:00 PM, Daniel Axtens notifications@github.comwrote:

No, a git pull won't overwrite, it will do a merge.

— Reply to this email directly or view it on GitHubhttps://github.com/daxtens/finvis/issues/2#issuecomment-12687112.

daxtens commented 11 years ago

I have made some comments on the commit.

I am using Python 2.7

daxtens commented 11 years ago

Also, please include the issue number (#2) in your commit message. I will update the instructions to reflect this.

axtens commented 11 years ago

So is this issue completed, or are you telling me that there's more to do, or that what has been done is not complete?

daxtens commented 11 years ago

There's some changes to be made. If you go to the commit, you'll see that there are comments on it. https://github.com/daxtens/finvis/commit/ec5f875d17bc6068fef6e7909a9f851ab9690b0d

axtens commented 11 years ago

If I take "this." off line 139 (and turn it into var) or off 169, then the page fails to arrange the circles appropriately.

On Fri, Jan 25, 2013 at 1:19 PM, Daniel Axtens notifications@github.comwrote:

There's some changes to be made. If you go to the commit, you'll see that there are comments on it. ec5f875https://github.com/daxtens/finvis/commit/ec5f875d17bc6068fef6e7909a9f851ab9690b0d

— Reply to this email directly or view it on GitHubhttps://github.com/daxtens/finvis/issues/2#issuecomment-12688502.

daxtens commented 11 years ago

OK, remove the line with @private. The rest is good as is.

axtens commented 11 years ago

On 25/01/2013 2:13 PM, Daniel Axtens wrote:

OK, remove the line with @private https://github.com/private. The rest is good as is.

— Reply to this email directly or view it on GitHub https://github.com/daxtens/finvis/issues/2#issuecomment-12689424.

Committed and pushed.

daxtens commented 11 years ago

Yay!