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

Prefix some interfaces with "I" #782

Merged
merged 19 commits into from
Apr 6, 2015
Merged

Conversation

SeanKilleen
Copy link
Contributor

Attempting to pick up where @HCanber left off. I'll remove the WIP when I feel it is ready for review.

This is the first time I'm really building Akka on my machine and at the outset, 36-37 tests usually fail when run via NCrunch (mostly around needing multi-node tests environments and such). So, my benchmark for commits will be that:

  • The project builds
  • The changes pass a sanity spot-check
  • No more than 37 tests fail

Below is my target list of interfaces, per @HCanber's comment on #652 and what he hadn't hit yet:

  • Akka.Actor.LoggingFSM, Akka (Public)
  • Akka.Actor.NoSerializationVerificationNeeded, Akka (Public)
  • Akka.Actor.PossiblyHarmful, Akka (Public)
  • Akka.Actor.WithBoundedStash, Akka (Public)
  • Akka.Actor.WithUnboundedStash, Akka (Public)
  • Akka.Dispatch.BlockingMessageQueueSemantics, Akka (Public)
  • Akka.Dispatch.BoundedDequeBasedMessageQueueSemantics, Akka (Public)
  • Akka.Dispatch.BoundedMessageQueueSemantics, Akka (Public)
  • Akka.Dispatch.DequeBasedMailbox, Akka (Public) (was already completed)
  • Akka.Dispatch.DequeBasedMessageQueueSemantics, Akka (Public)
  • Akka.Dispatch.MessageQueues.MessageQueue, Akka (Public)
  • Akka.Dispatch.MultipleConsumerSemantics, Akka (Public)
  • Akka.Dispatch.RequiresMessageQueue, Akka (Public)
  • Akka.Dispatch.Semantics, Akka (Public)
  • Akka.Dispatch.SysMsg.SystemMessage, Akka (Public)
  • Akka.Dispatch.UnboundedDequeBasedMessageQueueSemantics, Akka (Public)
  • Akka.Dispatch.UnboundedMessageQueueSemantics, Akka (Public)

Let me know if there's a more preferred way to structure this.

@Aaronontheweb
Copy link
Member

@SeanKilleen would you mind rebasing this PR on dev? The interface changes are one of those changesets that create conflicts pretty easily :p

@SeanKilleen
Copy link
Contributor Author

That's definitely my intention --was going to do so after I'd progressed through a few more of them.

I'm honesty newer to that process. Is my thinking okay or is it causing conflicts in the meantime? Don't want to hamper anyone else. Should I rebase as I go?

Thanks for any guidance. Additionally if it would be easier for me to back out and leave this to others, feel free to close the PR -- I won't be offended. :)

@Aaronontheweb
Copy link
Member

Sometimes I just do these merges myself, but lately I've been short on time :\

@SeanKilleen
Copy link
Contributor Author

@Aaronontheweb totally understandable -- I'm excited for the session at dotNetFringe next weekend!

I'm fine with however you'd like to do it, I'm not not used to contributing so if you have any guidance for how you'd like me to proceed with this chunk of commits, I'm happy to hear it.

Would you prefer that I make all of the small commits for all the checkboxes, then rebase, then remove WIP and we can look at merging? Or is there another strategy you'd like me to take, such as for example making each of these a separate simpler pull request?

Happy to do my best to help out -- just want to make sure I'm doing more help than harm to the productivity of the group.

@SeanKilleen
Copy link
Contributor Author

Sorry, I realize I may have misunderstood -- you meant for me to merge dev into this branch currently, I think. Correct? (if so that's totally fine, no problem)

@Aaronontheweb
Copy link
Member

Basically what I meant is bring in the latest changes sitting on the dev branch into the branch on this PR, so that way the "Merge pull request" button turns green ;p

@SeanKilleen
Copy link
Contributor Author

@Aaronontheweb quick question -- some of these tests that are failing appear to be tests that have nothing to do with my edits (in this case, DefaultResizer_must_grow_as_needed_under_pressure().

In this case, should I ignore the failure, or would you prefer me to re-run the tests somehow?

@Aaronontheweb
Copy link
Member

@Aaronontheweb
Copy link
Member

The Resizer / ConsistentHashRouter tests are known to be racy, as are some of the inbox specs. Please go through all failures and carefully review them.

@SeanKilleen
Copy link
Contributor Author

👍 will do.

@SeanKilleen
Copy link
Contributor Author

@Aaronontheweb I figured out the stashing issue for the most part I believe -- looks to be that I hadn't updated Pigeon.conf with the new values. Going to commit a few more changes so that I can then update all the pigeon.conf values at once and get the stash test green.

@Aaronontheweb
Copy link
Member

@SeanKilleen excellent - good work!

Aaronontheweb added a commit that referenced this pull request Apr 6, 2015
Work in Progress: Prefix some interfaces with "I"
@Aaronontheweb Aaronontheweb merged commit 0114148 into akkadotnet:dev Apr 6, 2015
@SeanKilleen
Copy link
Contributor Author

Checking off Semantics because it was already done.

@SeanKilleen
Copy link
Contributor Author

@Aaronontheweb now that this has been merged, I assume I have to create another PR to finish it off, correct?

@SeanKilleen SeanKilleen changed the title Work in Progress: Prefix some interfaces with "I" Prefix some interfaces with "I" Apr 7, 2015
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.

2 participants