debugger22 / github-audio

Listen to music generated by events happening across GitHub :octocat: 🎷
https://github.audio
MIT License
1.74k stars 99 forks source link

Mixed improvements on server and client #27

Closed mcornella closed 8 years ago

mcornella commented 8 years ago

This PR contains the following improvements:

Tested locally.

debugger22 commented 8 years ago

That was quick! 😄

mcornella commented 8 years ago

I was waiting for it :blush:

On another topic, these binaries are huge, it'd be better putting them in another place: maybe using github LFS or just putting them as a release which doesn't involve any additional installation.

In any case that would involve rewriting history to delete them out of the repository, because it has become super-slow to fetch and push due to the extra 80MB. It would probably involve telling people that you rebased master, but since it's 8 days ago it shouldn't have much impact and the benefit is clear.

debugger22 commented 8 years ago

Just one catch here. Now push events are redirecting to https://api.github.com URL.

debugger22 commented 8 years ago

Maybe that's fine, I'm not sure what users prefer.

mcornella commented 8 years ago

Just one catch here. Now push events are redirecting to https://api.github.com URL.

Oh sorry I thought I fixed it. It seems I didn't double check it and the API docs are wrong, there doesn't seem to be any html_url field:

captura de pantalla de 2016-10-21 09-37-02

{ id: '4745514621',
  type: 'PushEvent',
  actor: 
   { id: 7099784,
     login: 'docbacardi',
     display_login: 'docbacardi',
     gravatar_id: '',
     url: 'https://api.github.com/users/docbacardi',
     avatar_url: 'https://avatars.githubusercontent.com/u/7099784?' },
  repo: 
   { id: 70711456,
     name: 'muhkuh-sys/org.muhkuh.lua-jonchki',
     url: 'https://api.github.com/repos/muhkuh-sys/org.muhkuh.lua-jonchki' },
  payload: 
   { push_id: 1360737728,
     size: 5,
     distinct_size: 5,
     ref: 'refs/heads/master',
     head: '13ab152ce613758121339bd3a5b0ff1bea5d9790',
     before: '35d914f64b3d61b65d66859a3e4687b339bd4428',
     commits: [ [Object], [Object], [Object], [Object], [Object] ] },
  public: true,
  created_at: '2016-10-21T07:41:23Z',
  org: 
   { id: 7113795,
     login: 'muhkuh-sys',
     gravatar_id: '',
     url: 'https://api.github.com/orgs/muhkuh-sys',
     avatar_url: 'https://avatars.githubusercontent.com/u/7113795?' } }

It should be easy to change just manually concatenating the repo_name, as it was before in the client side:

diff --git a/server/index.js b/server/index.js
index caa879e..36b0774 100755
--- a/server/index.js
+++ b/server/index.js
@@ -117,7 +117,7 @@ function stripData(data){
           'payload_size': data.payload.size,
           'message': data.payload.commits[0].message,
           'created': data.created_at,
-          'url': data.repo.url
+          'url': 'https://github.com/' + data.repo.name
         });
         pushEventCounter++;
       }
debugger22 commented 8 years ago

Thanks! I'll do it.

mcornella commented 8 years ago

Make sure to remove the binaries, it will be a huge source of pain in the future, that happened to me as well. git rebase and delete the commits, don't make a git rm commit because the size will still be there in the earlier commits. Right now not many forks have these commits so it'll be easy making a push force. Use github releases to store the binaries instead.

debugger22 commented 8 years ago

Did you mean to remove the binary files and rebase to the commit prior to the commit having binary files?

mcornella commented 8 years ago

No, that won't work. You have to make an interactive rebase to the commit prior to the one having binary files. git rebase -i 864d3ea74^ and edit the commits where you introduced them.

You can use git log --oneline --name-only to identify which files changed on each commit:

$ git log --oneline --name-only 
212260b Merge pull request #27 from mcornella/mixed-improvements
c5a04f3 Add URL field for each event
app/public/js/main.js
server/index.js
5301419 Set `merged' action if `merged' key is true
server/index.js
9ccb7a3 Add cross-env dependency so that `npm start' works
package.json
bdb0b48 Merge pull request #31 from jennylia/master
5be2b66 added some details to README
.gitignore
README.md
app/public/js/main.js
1a3be3a Update index.html
app/index.html
53961da Add an extra line break for apps text
app/index.html
ecae20e Add app links to website
app/index.html
afaa324 Move bin to static
app/public/bin/GitHub-Audio-v1.0-linux-x64.zip
app/public/bin/GitHub-Audio-v1.0-macOS-x64.zip
864d3ea Add binaries for macOS and Linux
app/index.html
app/public/css/main.css
bin/GitHub-Audio-v1.0-linux-x64.zip
bin/GitHub-Audio-v1.0-macOS-x64.zip
0d9e7ac Fix draw issue on hidden screen
app/public/js/main.js
684dd31 Fix syntax error

It seems you'll only have to edit 864d3ea (Add binaries for macOS and Linux) and afaa324 (Move bin to static). If you're unfamiliar with rebase interactive let me know, but there should be a helpful explanation of how to use it once you run the git rebase -i command.

Let me know if I need to explain further!

Oh, and at some point you'll have to also change the URLs from index.html but that can be done afterwards.

mcornella commented 8 years ago

I've found a way to do it programatically better. Make sure you're on the most updated master and you have your zip files backed up somewhere else:

  1. Reconstruct git history ignoring big files (from Purging a file or directory from history):

    reconstruct_git_ignoring_file() { git filter-branch -f --prune-empty --index-filter \
    "git rm -rf --cached --ignore-unmatch \"$1\"" --tag-name-filter cat -- --all }
    
    reconstruct_git_ignoring_file app/public/bin/GitHub-Audio-v1.0-linux-x64.zip
    reconstruct_git_ignoring_file app/public/bin/GitHub-Audio-v1.0-macOS-x64.zip
    reconstruct_git_ignoring_file bin/GitHub-Audio-v1.0-linux-x64.zip
    reconstruct_git_ignoring_file bin/GitHub-Audio-v1.0-macOS-x64.zip
  2. Delete backup refs, purge and garbage-collect all unreachable commits: those in the master branch, other branches or on tags won't be deleted (from this stackoverflow answer):

    git for-each-ref --format='delete %(refname)' refs/original | git update-ref --stdin
    git reflog expire --expire-unreachable=now --all
    git gc --prune=now

This reduces significantly the size of the git repository:

$ du -s .git   # before
83M     .git
$ du -s .git   # after
2.5M    .git

Afterwards, you have to make a force push git push -f and that's all. Make sure you merge #26 first or else it will be more difficult solving the conflict.