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

Add AnnotateRb gem #1145

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Add AnnotateRb gem #1145

merged 2 commits into from
Nov 26, 2024

Conversation

drwl
Copy link
Contributor

@drwl drwl commented Aug 18, 2024

Project

Adds AnnotateRb gem to the list of gems.

Rubygems
Github repo

What is this Ruby project?

It's an active and maintained hard fork of the old Annotate gem. It improves the developer experience by adding model database schemas as text comments in corresponding ActiveRecord models. It also supports annotating routes.

What are the main difference between this Ruby project and similar ones?

It's actively maintained compared to it's predecessor, the Annotate gem.

@markets
Copy link
Owner

markets commented Aug 26, 2024

Hey @drwl 👋🏼 thanks for this!

Maybe we should replace (delete ✂️) the original repo too? Would be nice to wait for some official answer 👀 ctran/annotate_models#1028.

@drwl
Copy link
Contributor Author

drwl commented Aug 26, 2024

I don't know if/when we will get an official answer, there's ctran/annotate_models#1001 from earlier this year with also no answer.

As to whether or not to replace the original repo, I believe that's your decision. I can adjust the PR to delete it if you want.

@markets
Copy link
Owner

markets commented Aug 26, 2024

Yep, I've already seen that earlier issue too, but would be nice any kind of "official" movement/message from the original maintainers. For such relevant and used gems, it's much better any official communication to avoid confusions within the Ruby ecosystem (specially for new comers). Did you try to contact the original maintainer? If they don't want to maintain it anymore, at least a message in the README (pointing to your fork) would help a lot!

In terms of this PR, I'd also prefer to have only one resource, with the goal to make it clear for all the people. Having both links in this collection it seems that creates again confusion for users checking this list.

@drwl
Copy link
Contributor Author

drwl commented Aug 26, 2024

For such relevant and used gems, it's much better any official communication to avoid confusions within the Ruby ecosystem (specially for new comers).

Totally agree there.

Did you try to contact the original maintainer?

If they don't want to maintain it anymore, at least a message in the README (pointing to your fork) would help a lot!

Yes I have tried contacting in the past and no response. Also, an even earlier Github thread/issue.

ctran/annotate_models#913

In terms of this PR, I'd also prefer to have only one resource, with the goal to make it clear for all the people.

I can modify the PR to delete the old one, let me know if that's what you mean.

@markets
Copy link
Owner

markets commented Aug 26, 2024

Yes I have tried contacting in the past and no response

Ok! then let's do it! Your fork seems that is the one that people is using now, so it seems the better option to propose at this point 🤔? And the community already tried, for more than 2 years, to contact the original authors with no luck:

I can modify the PR to delete the old one, let me know if that's what you mean.

Yes, I think having only 1 link it's clearer for everybody visiting this collection. Also, since you already have a comment about the fork in your README, I'd like to suggest to remove last sentence in the proposed description.

@markets markets removed the addition label Aug 26, 2024
@marcoroth
Copy link
Contributor

marcoroth commented Aug 26, 2024

FWIW, I've been using the annotate gem for a long time and haven't had any issue with it, also not in recent times. I think it's fair to say that it could be considered "done" from the maintainers POV.

But I also see there are a lot of open issues and new feature requests. @drwl have you contacted the RubyGems team to see if they can do something about it? I think they might be able to help with that, so we don't have to have forks, which are also very similarly named, which adds a lot of confusion in my opinion.

@markets
Copy link
Owner

markets commented Aug 26, 2024

Thanks for jump in @marcoroth! I'm also using the original one in a couple of small apps (Rails 6.1 and 7.0) with no issues at all. But I didn't introduced DB changes for months... so I'm really not sure about the current maintenance status of the original repo.

Maybe it's better idea to wait for any kind of official communication before moving forward and merge this PR now? Would be nice to talk about it with the current maintainers.

I'd like to avoid having both resources in the list, that will be confusing for people looking into it.

@drwl
Copy link
Contributor Author

drwl commented Aug 27, 2024

I think it's fair to say that it could be considered "done" from the maintainers POV.

Calling it "done" is an understandable opinion, and for most setups it is. The core functionality should work, because all the gem really does is find model files and reflect on their ActiveRecord columns. I would say there are some outstanding usability bugs in the old gem like swallowing comments, but if you don't come across that in your workflow then there really isn't a need to upgrade at this time (note: future Rails versions/changes could change things).

@drwl have you contacted the RubyGems team to see if they can do something about it? I think they might be able to help with that, so we don't have to have forks, which are also very similarly named, which adds a lot of confusion in my opinion.

Just curious, has there been precedent about this? Afaik there's no imminent security risk of the old gem being up, so not sure why they would do anything about it. As an example, the old CanCan gem is still on Rubygems even though it was community forked years ago. My gem/fork also is a hardfork in that it uses a new namespace.

Maybe it's better idea to wait for any kind of official communication before moving forward and merge this PR now? Would be nice to talk about it with the current maintainers.

Ultimately it's your decision regarding the awesome ruby list. Given that the oldest out of the 3 issues is 3 years old, I don't see anything changing in the coming years.

@marcoroth
Copy link
Contributor

marcoroth commented Aug 27, 2024

@drwl I was more alluding to the fact that they might be able to help you get ownership over the gem so you could continue maintaining and releasing it under the "old" name. But I guess this is too late already, since you have your fork and gem already.

@drwl
Copy link
Contributor Author

drwl commented Aug 27, 2024

Ahh got it, yeah I changed the namespace (and the name) because there were some big changes that I wanted to make (e.g. removing config in Rakefile) and thought it made more sense. Also, to signify that there were big breaking changes between the old gem and annotaterb.

@markets markets mentioned this pull request Oct 29, 2024
@drwl
Copy link
Contributor Author

drwl commented Oct 30, 2024

I got an email from Github about the PR -- just checking is there anything needed on my end? The last PR in the old repo was merged in almost a year ago.

https://github.com/ctran/annotate_models/commits/develop/

@markets
Copy link
Owner

markets commented Oct 30, 2024

Not really, I'm still not sure what's the best move here ...

  • Having both resources seems like an exception and confusing for users checking this repo. I'd like to avoid this.
  • Yours seems more up-to-date and with new features.
  • But the original still works for me without any problem and it's like the canonical for most of the community.

So, I have no idea what's the best decision here. Let's leave it open for now, maybe some day we'll resolve these doubts.

@markets
Copy link
Owner

markets commented Nov 20, 2024

It seems that community is moving to @drwl's fork: ctran/annotate_models#1032.

So maybe it's time to remove the old link and merge this PR? With the recent Rails v8 release it looks that people are just migrating to that fork, which is compatible with Rails v8.

@markets
Copy link
Owner

markets commented Nov 22, 2024

@drwl Could you please delete the original project link? I think it's time to merge this one.

@markets markets merged commit 6fdf72a into markets:master Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants