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.

Sign up for GitHub

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

Jump to bottom

Refactor json #98

Merged
merged 15 commits into from
Feb 23, 2020
Merged

Refactor json #98

merged 15 commits into from
Feb 23, 2020

Conversation

eloisepeng
Copy link
Collaborator

@eloisepeng eloisepeng commented Feb 17, 2020 •
edited by chuck-sys
Loading

🚢 Pull Request

❓ Purpose

Give a brief description of your changes:

  • refactor code and turn all events/choices into JSON
  • added new PickResidence event (made by @tonywu94)
  • leave NoMoreEvent in code (removed from .json)

🔨 Validation Strategy

Describe how you verified your changes (if applicable):

🎟️ Ticket(s)

Closes #84
Closes #99

@eloisepeng eloisepeng requested a review from a team as a code owner February 17, 2020 12:28
@eloisepeng eloisepeng mentioned this pull request Feb 17, 2020
Closed
chuck-sys
chuck-sys previously approved these changes Feb 20, 2020
View reviewed changes
Copy link
Contributor

@chuck-sys chuck-sys left a comment

Choose a reason for hiding this comment

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

Looks good! One thing to note is that we may want to note down in the documentation that events.NoMoreEvents is a special type of event that must be included or else this thing won't compile.

rmcreyes
rmcreyes requested changes Feb 20, 2020
View reviewed changes
Copy link
Collaborator

@rmcreyes rmcreyes left a comment

Choose a reason for hiding this comment

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

Looks great! Just a few nits.

src/events/EventsManager.ts Show resolved Hide resolved
src/App.tsx Outdated Show resolved Hide resolved
@eloisepeng eloisepeng requested a review from rmcreyes February 22, 2020 22:45
rmcreyes
rmcreyes previously approved these changes Feb 22, 2020
View reviewed changes
@eloisepeng eloisepeng merged commit 4083afe into master Feb 23, 2020
@eloisepeng eloisepeng deleted the refactor-json branch February 23, 2020 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmcreyes rmcreyes rmcreyes approved these changes

@chuck-sys chuck-sys chuck-sys approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

move noMoreEvent into the code Turn events from classes to JSON
3 participants
@eloisepeng @chuck-sys @rmcreyes