LibraryOfCongress / bagger-js

Upload BagIt-format deliveries to S3 entirely in the browser
https://libraryofcongress.github.io/bagger-js/
Other
32 stars 7 forks source link

Cleaning up / adding Redux #31

Closed eikeon closed 8 years ago

eikeon commented 8 years ago
johnscancella commented 8 years ago

what is @houndci-bot and why is it spamming this PR?

acdha commented 8 years ago

@johnscancella See https://houndci.com/ – it's a style-checker. Unfortunately, the GitHub API doesn't allow you to make multiple line-level comments at once.

acdha commented 8 years ago

I just made a commit to have it honor the .jshintrc so it gets the esnext option:

https://github.com/LibraryOfCongress/bagger-js/commit/6107e2478dc41e26e58eea8f160f73a4a9e23ad2

eikeon commented 8 years ago

Looks like all the hound notes stick around even after a push -f

Should I close this pull request and open a new one to start clean? Thoughts, @acdha ?

acdha commented 8 years ago

@eikeon You can delete anything which was just caused by the missing esnext option

eikeon commented 8 years ago

Since they didn't seem to go away after I did a push -f ... I deleted them by hand.

acdha commented 8 years ago

@eikeon It looks like the eslint switch was the third of four commits, which would explain some of this

eikeon commented 8 years ago

@acdha It's the first commit in the repo from which I: push -f

acdha commented 8 years ago

I've never seen that not update – do the commit hashes match what's on https://github.com/LibraryOfCongress/bagger-js/pull/31/commits?

eikeon commented 8 years ago

Looks like it:

git log --oneline 3a84655 Updated re: babel/babel-eslint#267 21531be Removed stray bits dbcc7e0 Cleaning up / adding Redux 9ec6467 Disable jshint; enable eslint to align with Gulp 6107e24 Create .hound.yml

eikeon commented 8 years ago

Thinking of https://facebook.github.io/fixed-data-table/ for an updated bag content presentation

acdha commented 8 years ago

As a topic to save for later, the editorconfig, etc. is making me wonder whether we should try esformatter with the JSX module so there's a simple way to autoformat everything.

eikeon commented 8 years ago

:+1: to looking into adding more tooling around formatting. We'll want something that understands the babel we're speaking. Currently I'm relying mostly on: https://atom.io/packages/language-babel

eikeon commented 8 years ago

Think I'm missing the mark on these top level state names: bagger, hasher, and uploader? At a minimum the bag -> bagger seems to have missed.