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

Continuing Roodi Development #16

Merged
merged 3 commits into from Aug 14, 2013
Merged

Continuing Roodi Development #16

merged 3 commits into from Aug 14, 2013

Conversation

bf4
Copy link
Contributor

@bf4 bf4 commented May 19, 2013

Hey, if you're continuing roodi development #13 I'd love to help out. For review, here are the modifications in my fork that I am using for running metric_fu. If you like, I can remove the namespace-changing commits and any others not not necessary.

Thanks

@bf4
Copy link
Contributor Author

bf4 commented Aug 2, 2013

I'm now maintaining a metric_fu-roodi fork

@evjan
Copy link
Contributor

evjan commented Aug 8, 2013

Hi, sorry about the delay.

Pull request are very welcome! Are all of these still relevant?

@bf4
Copy link
Contributor Author

bf4 commented Aug 8, 2013

Wait, are you still planning on maintaining roodi here? I just switched 'official' development to the metric_fu fork. Are they relevant? I'm using them.

@evjan
Copy link
Contributor

evjan commented Aug 8, 2013

I'm not the previous maintainer. I volunteered to take over the ownership of Roodi from today, since it hadn't received much love lately.

What's the best way of moving forward in your opinion?

@ghost ghost assigned evjan Aug 8, 2013
@bf4
Copy link
Contributor Author

bf4 commented Aug 9, 2013

Well, if you're now maintaining it, then I'd love to roll this stuff in and help you maintain it here. The PR is what I gathered from forks while roodi languished and I began maintaining for metric_fu. What awful timing.

See the history here, too https://github.com/metricfu/roodi/commits/master

@bf4
Copy link
Contributor Author

bf4 commented Aug 9, 2013

In particular, locking to ruby_parser 2.3 is problematic 66bc3c5 cc: @dkubb

@bf4
Copy link
Contributor Author

bf4 commented Aug 9, 2013

I'd honestly rather make metricfu the official sponsoring organization, pull in any relevant work you've recently done here, and continue over there.

@bf4
Copy link
Contributor Author

bf4 commented Aug 9, 2013

Also, I have a rubyforge account if you want any help / assistance updating http://roodi.rubyforge.org/ to redirect here

@evjan
Copy link
Contributor

evjan commented Aug 11, 2013

Yeah, the timing was bad. Sorry about that, but let's make the best of it.

Regarding moving to metricfu as the organisation: Given that some people are using Roodi without using Metricfu, I'd rather create a new organisation called Roodi and move this repo there instead. That way it is clear that it is not dependent on Metricfu or belongs to it (since Metricfu rather depends on it), but also that it is not tied to a person who is no longer maintaining it. I'll take a look at that asap and then we'll start merging our changes.

@bf4
Copy link
Contributor Author

bf4 commented Aug 12, 2013

On Sun, Aug 11, 2013 at 6:13 PM, Peter Evjan notifications@github.comwrote:

Yeah, the timing was bad. Sorry about that, but let's make the best of it.

Regarding moving to metricfu as the organisation: Given that people are
using Roodi without using Metricfu, I'd rather create a new organisation
called Roodi and move this repo there instead. That way it is clear that it
can be run without Metricfu, but it is not tied to a person who is no
longer maintaining it. I'll take a look at that asap and then we'll start
merging our changes.

perfect. It's super easy. You create a github organization, then probably
want to fork a repo there and update his to say 'go there' :) In the
github org, you can have teams so that, for example, you can control the
org, and put me on a team called 'roodi maintainers' that has me on it as a
commiter, then add 'roodi' to that team.

-Benjamin

@evjan
Copy link
Contributor

evjan commented Aug 12, 2013

I did that, only to discover there might be a better way: transferring the repo.

I'll just have a chat to Marty before doing that.

@bf4
Copy link
Contributor Author

bf4 commented Aug 12, 2013

On Sun, Aug 11, 2013 at 9:06 PM, Peter Evjan notifications@github.comwrote:

I did that, only to discover there might be a better way: transferring
the repo https://help.github.com/articles/how-to-transfer-a-repository

.

I'll just have a chat to Marty before doing that.

I believe github now offers redirect, so that is an option without side
effects. MetricFu's previous maintainer preferred to keep his repo there
for historical purposes, so that's where I was coming from.

@evjan
Copy link
Contributor

evjan commented Aug 12, 2013

I don't see an option to redirect to another org without transferring it to that org. See here https://github.com/blog/1508-repository-redirects-are-here

@bf4
Copy link
Contributor Author

bf4 commented Aug 12, 2013

Yes, @evjan, that's what I meant, sorry for being unclear.

@bf4
Copy link
Contributor Author

bf4 commented Aug 12, 2013

Can you go into settings for https://github.com/roodi/roodi and enable issues? I'll disable issues in metricfu/roodi and update to point there shortly. I wanted to create an issue to reference this PR.

@evjan
Copy link
Contributor

evjan commented Aug 13, 2013

I've now transferred the repo to the Roodi org. You should be able to create issues now.

Marty and I paired on getting it working with the latest version of ruby_parser and once all those tests are green I'll review your pull requests.

evjan pushed a commit that referenced this pull request Aug 14, 2013
@evjan evjan merged commit 0dd9c64 into roodi:master Aug 14, 2013
@evjan
Copy link
Contributor

evjan commented Aug 14, 2013

I've merged this, thanks a lot for this!

A few questions:
e659916 - there were no specs for this, when does it happen? I'd love to have specs for all functionality.
9119a3d - What's the benefit of this? I see no difference when running rake with and without it? So removed it for now.

@bf4
Copy link
Contributor Author

bf4 commented Aug 19, 2013

Re: e659916 I don't recall, besides that it crashed. Oh well.

Re: 9119a3d. I don't recall. Probably from a template I was using. If it's not needed, for sure to remove.

@evjan
Copy link
Contributor

evjan commented Aug 19, 2013

Thanks for the feedback! I removed e659916 in 5112286 as I could not provoke it through the specs. The crash is probably handled through the new count_lines logic.

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.

None yet

2 participants