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

Linked minutes #325

Merged
merged 3 commits into from Jan 21, 2015
Merged

Linked minutes #325

merged 3 commits into from Jan 21, 2015

Conversation

mikaelbr
Copy link
Contributor

It seems as it isn't possible to have linked graphs on anything other than days, due to the hardcoded parsing for dates. This PR adds possibility to override this format, and thus being able to do linked hover on self defined granularity.

This PR also adds convenience npm scripts for building and testing:

$ npm run build
$ npm test

This makes it so that gulp doesn't have to be installed globally and thus makes it easier to have different gulp version across different projects.

@mikaelbr
Copy link
Contributor Author

I see. It seems as the tests doesn't run with the old script though? It runs, but with 0 tests. I reverted the commit and squashed them

@dandehavilland
Copy link
Contributor

It seems as the tests doesn't run with the old script though?

They're running for me locally and via travis with the current setup.

If installing gulp globally is not a viable solution, then we could reconfigure .travis.yml and package.json. Something like this:

# .travis.yml
...
script: npm run test-ci
...
# package.json
...
"scripts": {
  "build": "gulp build:js",
  "test": "gulp test",
  "test-ci": "./node_modules/testem/testem.js ci testem.json"
},
...

@mikaelbr
Copy link
Contributor Author

I think that sounds like a good compromise. Installing gulp globally works as well, but it's considered bad practice IMO. I can update the commit.

@mikaelbr
Copy link
Contributor Author

For some reason, when i run ./node_modules/testem/testem.js ci testem.json i get the following:

➜  metrics-graphics git:(linkedMinutes) ✗ npm run test-ci

> metrics-graphics@2.0.0 test-ci /projects/repos/metrics-graphics
> ./node_modules/testem/testem.js ci testem.json


1..0
# tests 0
# pass  0
# fail  0

# ok

@dandehavilland
Copy link
Contributor

I don't know why that is. I ran ./node_modules/testem/testem.js ci testem.json on your branch and get:

1..80
# tests 80
# pass  80
# fail  0

# ok

@mikaelbr
Copy link
Contributor Author

Strange. It seems to work when opening in the browser (running gulp test and opening the address.). Might be a headless browser problem. I'll try to investigate more later today.

@dandehavilland
Copy link
Contributor

Ah yes, do you have PhantomJS installed?

@mikaelbr
Copy link
Contributor Author

Ah. I didn't have it installed globally, so it didn't work as expected. Installed it globally now and it shows 80 tests.

@dandehavilland
Copy link
Contributor

Cool, looks good. Will now just need a rebase against master.

@mikaelbr
Copy link
Contributor Author

Maybe also the linked_format property should be in the defaults?

@almossawi
Copy link
Contributor

Thanks, @mikaelbr 👍

@hamilton @dandehavilland Looks good to you?

@dandehavilland
Copy link
Contributor

Yep, looks good to me.

almossawi added a commit that referenced this pull request Jan 21, 2015
@almossawi almossawi merged commit 782ec78 into metricsgraphics:master Jan 21, 2015
@almossawi almossawi added this to the v2.1 milestone Jan 21, 2015
@mikaelbr mikaelbr deleted the linkedMinutes branch January 21, 2015 16:03
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

Successfully merging this pull request may close these issues.

None yet

3 participants