Skip to content
This repository has been archived by the owner on Jul 13, 2018. It is now read-only.

Add BaseStore and BaseComponent. Add GitHub PRs. #15

Merged
merged 2 commits into from
Apr 24, 2015
Merged

Conversation

mythmon
Copy link
Owner

@mythmon mythmon commented Apr 23, 2015

This is pretty big, and wandered a bit, unfortunately.

I added a new store to correlate bug and PR data. I'm not 100% sure this is the best way to approach it, but it seems to work.

I added a BaseStore and a BaseComponent to factor some common behavior.

I also struggled a bunch with Immutable.js. I think I'm getting the hang of it. Going forward, I think that that all data that comes out of the stores should be Immutable.js objects, but all other data in the system should be normal JS objects. I think that means that the components only deal in Immutable objects too. That's good, later we can use it to make the UI go super fast. The stores are the place where native JS objects become Immutable JS objects.

I've maintained, I think, pretty good test coverage. It has been useful. I'd like to explore how to test the components.

@mythmon mythmon force-pushed the master branch 2 times, most recently from af21a04 to 9a57f19 Compare April 24, 2015 17:18
</td>
<td className="BugTable__data--number">
{this.props.whiteboard_parsed.p}
{prs.map((pr) => <a href={pr.get('html_url')}>#{pr.get('number')}</a>)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking about how we do PRs in ernest and instead of just showing the currently open prs, maybe we should instead show all the prs for this bug and some indication of their state (open/closed).

I don't think we need to change that in this pr, but it's something I've been thinking about.

Copy link
Owner Author

Choose a reason for hiding this comment

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

That sounds nice. I'm not quite sure how to efficiently get closed bugs too though, without making lots and lots of GitHub queries (one per bug). Unlike Bugzilla, GitHub will eventually rate limit us.

@willkg
Copy link
Collaborator

willkg commented Apr 24, 2015

This looks fine as far as I can tell. I need to spend more time on learning flux, but I doubt I can do that enough of that today to have more meaningful thoughts.

I think instead we should just plunge ahead and write edwin with the expectation that we might hit a point where we want to rewrite large portions based on what we learned.

mythmon added a commit that referenced this pull request Apr 24, 2015
Add BaseStore and BaseComponent. Add GitHub PRs.
@mythmon mythmon merged commit f679736 into master Apr 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants