Skip to content

Conversation

@randy-girard
Copy link
Contributor

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should include validation / appropriate error messages here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a few things:

  • Tests
  • Minor bump.
  • Update CHANGELOG

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also looking in v3, I don't see this command. Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@randy-girard
Copy link
Contributor Author

@matthopson Added some tests, validations, and changelog updates.

CHANGELOG.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to bump to 1.8.0 since API change.

Will also want to update in package.json and teamsnap.coffee

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@matthopson
Copy link
Contributor

Just a side-note, it's probably a good idea to include a note + reference to the v3 PR so it's clear it hasn't been merged yet (since the v3 PR needs to be merged before this can be).

@randy-girard randy-girard changed the title Notify team of events import WIP: Notify team of events import Jan 25, 2016
@randy-girard randy-girard force-pushed the notify-team-of-events-import branch from 82e278b to b8f5a74 Compare February 4, 2016 15:29
@randy-girard randy-girard changed the title WIP: Notify team of events import Notify team of events import Feb 4, 2016
@randy-girard randy-girard changed the title Notify team of events import Bulk create and notify events Feb 4, 2016
@matthopson
Copy link
Contributor

One more small thing. Would you mind updating https://github.com/teamsnap/teamsnap-javascript-sdk/blob/master/docs/collections/events.md with this?

@randy-girard
Copy link
Contributor Author

@matthopson
Copy link
Contributor

👍 loop it...

johnny-mnemonic-internet1

@randy-girard randy-girard force-pushed the notify-team-of-events-import branch from a0ebb60 to 6905a05 Compare February 4, 2016 15:46
randy-girard added a commit that referenced this pull request Feb 4, 2016
@randy-girard randy-girard merged commit 951be22 into master Feb 4, 2016
@randy-girard randy-girard deleted the notify-team-of-events-import branch February 4, 2016 15:46
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.

3 participants