Skip to content
This repository was archived by the owner on Jan 12, 2020. It is now read-only.

Consolidate projects and break out Config#59

Merged
mgodave merged 3 commits into
masterfrom
config_breakout
Feb 7, 2014
Merged

Consolidate projects and break out Config#59
mgodave merged 3 commits into
masterfrom
config_breakout

Conversation

@mgodave

@mgodave mgodave commented Feb 7, 2014

Copy link
Copy Markdown
Owner
  • Consolidate barge-rpc-proto and barge-service into one project
  • Break out ClusterConfig and Replica into interfaces so implementations can define their own behavior

* Consolidate barge-rpc-proto and barge-service into one project
* Break out ClusterConfig and Replica into interfaces so implementations can define their own behavior

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not needed in the interface declaration?

@abailly

abailly commented Feb 7, 2014

Copy link
Copy Markdown
Collaborator

Minor comments here and there but mostly LGTM.

@mgodave

mgodave commented Feb 7, 2014

Copy link
Copy Markdown
Owner Author

I'll fix these all and merge the PR

@justinsb

justinsb commented Feb 7, 2014

Copy link
Copy Markdown
Collaborator

All these refactorings are going to make it harder to get the critical missing features into barge. Code organization doesn't matter if the library isn't used :-)

Do you think we can have a "refactoring" phase, and then we can plan a "feature" phase. No major refactorings during the feature phase, until we get (1) start/stop (@justinsb), (2) dynamic reconfiguration (@justinsb) and (3) snapshots / log truncation (@mgodave ?)

Otherwise I just can't keep up with you guys!

@abailly

abailly commented Feb 7, 2014

Copy link
Copy Markdown
Collaborator

I totally agree that "code organization doesn't matter if the library isn't used". However I do not agree that refactoring should be done independently of feature development. Actually, the whole point of refactoring is to be able to ship features more quickly and efficiently. As Martin Fowler (the guy who coined the term "refactoring") puts it, the only motivation of refactoring should be the economics of the project.

I initiated some changes because I want to use barge, and quickly. However I want to use a library I understand and I can trust because I want to concentrate on the things I can do with barge. If something goes wrong, I want to be sure this is not some obscure bug in barge but something I did wrong or misunderstood. This implies 1/ clarity of the code and 2/ testing and rock-solid basic behaviour.

What's preventing you from submitting PRs? Maybe you can submit smaller PRs in a way that would allow you to develop those missing features while keeping up with other developments and possibly refactoring? git is pretty good at merging and rebasing stuff. Moreover, smaller PRs makes it easier to review and discuss changes, which increases the likehood of fast integration in the mainline.

My 50 cts.

@justinsb

justinsb commented Feb 7, 2014

Copy link
Copy Markdown
Collaborator

I have an outstanding pull request for dynamic reconfiguration #54 (too large, but it's a big feature). I know @mgodave has a branch somewhere where he's been working on log compaction.

I'm breaking up my large feature branch into smaller PRs, starting with the ability to start and stop the server #55. That's blocked on a bug in another project we use/used (netty-protobuf-rpc), so I had to go and fix it there.

Git is OK at rebasing minor changes, but it is much harder for everyone when the refactorings are large.

I'm not disputing the value of the refactorings - they're great. I'm just asking that we have a period where we just focus on getting in these X changes, and not do anything that makes that any harder.

@abailly

abailly commented Feb 7, 2014

Copy link
Copy Markdown
Collaborator

As I said, I do not feel I have the knowledge nor the authority to have insightful comments on #54. I already contributed to #55. I think that the latter is probably mergeable quickly given netty-rpc-protobuf has been fixed.

@mgodave

mgodave commented Feb 7, 2014

Copy link
Copy Markdown
Owner Author

@justinsb I apologize, I haven't been given your PRs the attention I should. I latched on to the refactoring efforts @abailly was doing because 1/ I think they are important 2/ I found implementing log compaction was needlessly hard and coupled without a lot of there refactoring efforts.

I understand that you're frustrated. I think you have some valid points and I don't want to invalidate your hard work. I also want you to keep using/contributing to this project.

All of that being said, I think the "major refactoring" efforts are coming to a head. Might I suggest a wiki page (you all have permissions to create/edit) where we can log what we think is needed/next. I'd like to get started on feature work again and get @justinsb's work off a long running branch. I'm also in favor of many smaller commits/PRs, I'm a frequent abuser of large commits.

So, can we:

  1. get a wiki going and move the discussion there?
  2. get the current in-flight refactors merged
  3. work on getting @justinsb's PRs up-to-date and merged
  4. I'll work on getting log compaction in

There is no reason we can't keep refactoring as we go. It might be nice to keep them small and keep the communication lines open.

@justinsb

justinsb commented Feb 7, 2014

Copy link
Copy Markdown
Collaborator

I think we're all on the same page here:

  • The refactorings are valuable
  • Big refactorings hurt long-running branches
  • Long running branches are bad, but it's also hard to do a bigger feature without one
  • We have some big features / branches we need to land
  • The big refactorings are (almost) done

So I think we can agree that we should be able to reach a point where we favor features over big refactoring for a week or two. Obviously small refactorings can go on, but we need the features.

I don't really know if we need a wiki - the two big missing features are the same two they've always been, and are pretty high in the README: log compaction & dynamic reconfiguration. But happy to put that on a wiki page!

@mgodave

mgodave commented Feb 7, 2014

Copy link
Copy Markdown
Owner Author

@justinsb I'm fine with (all) that.

@mgodave

mgodave commented Feb 7, 2014

Copy link
Copy Markdown
Owner Author

@justinsb feel free to ping me on google chat or email whenever (my email is on my profile)

mgodave added a commit that referenced this pull request Feb 7, 2014
Consolidate projects and break out Config
@mgodave mgodave merged commit a2e3437 into master Feb 7, 2014
@mgodave mgodave deleted the config_breakout branch February 11, 2014 16:36
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.

3 participants