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

question: Is ORM needed? #151

Closed
maiha opened this issue Feb 22, 2018 · 13 comments
Closed

question: Is ORM needed? #151

maiha opened this issue Feb 22, 2018 · 13 comments

Comments

@maiha
Copy link
Contributor

maiha commented Feb 22, 2018

Hi all, I'm simply wondering why it's named as Granite::ORM::Base.
For example, ActiveRecord is quite natural.

  • ActiveRecord::Base # base
  • ActiveRecord::ConnectionAdapters::PostgreSQLAdapter # and its adapters

In Granite

  • Granite::ORM::Base
  • Granite::Adapters::Pg

I prefer shorter naming like Granite::Base.

class User < Granite::Base

Furthermore, I prefer granite for the repository name if there are no plans to release any other granite-xxx projects.
Thought?

@drujensen
Copy link
Member

I am fine with this change and agree that ORM is not needed. We will either need to make it backward compatible or coordinate closely with amber cli so the templates are updated with the correct major version change.

@robacarp
Copy link
Member

I agree, but changing the repo name is kind of painful because it breaks all builds generated with the old repository name.

@maiha
Copy link
Contributor Author

maiha commented Mar 30, 2018

How about making a new repository named granite while leaving granite-orm?

  • granite-orm : exists for old applications, but should be readonly (no more issues and pr)
  • granite : a new repository used from the next version of amber like v0.7

Since there are only 2 bug issues and 3 PRs, I think that it is a good opportunity to execute now. 😺

@eliasjpr
Copy link
Contributor

What about Granite::Model instead of Granite::Base

class User < Granite::Model
end

@faustinoaq
Copy link
Contributor

I agree, but changing the repo name is kind of painful because it breaks all builds generated with the old repository name.

I think that's not a problem, github still redirects it for some time and amber is not v1.0.0 yet

I propose to change this repo name

BTW, I like Granite::Model as well 👍

@robacarp
Copy link
Member

I know Amber isn't 1.0 yet, but I don't think we should be in the habit of abandoning users. The redirect is actually going to make things worse because it stays around long enough for everyone to forget that the old one ever existed and when things break, it's harder to find out what happened.

If we want to change the name of the repo, here are the steps I'd recommend following:

  • Renaming this repo to granite will preserve the issues, pull requests, releases, etc.
  • Then, a new granite-orm can be created (via fork or whatever)
  • adding some macro code to granite-orm which will inform users that the dependency is out of date, that the shard is no longer maintained, and where to find the new replacement.
  • Updating the readme to provide instructions.
  • put granite-orm into Archive mode.
  • When Amber 1.0 is released, the old granite-orm repo can be deleted.

After the repo is renamed, changes to the the names of base classes and such can be done incrementally.

@faustinoaq
Copy link
Contributor

@robacarp I agree, you proposal is very nice 👍

@faustinoaq
Copy link
Contributor

@drujensen I really like the @robacarp proposal, I think it would do no harm, because the granite-orm repo will still be available, and this repo will be renamed to a simple name granite preserving issues, PR and more.

So, Can you change the name and create granite-orm in archive mode, following the steps mentioned above?

WDYT? 😅

@drujensen
Copy link
Member

@robacarp Excellent proposal. 👍

I will rename the repository now but let's hold off on the archived granite-orm repo until we get the current three/four PR's merged.

@robacarp
Copy link
Member

robacarp commented Apr 2, 2018

Thanks for renaming the repository, I think that paves the way for improvements. I'm going to go ahead and created "the old" granite repo again as granite-orm and pushed code up to it. Without it, I think a lot of builds are broken. Any pull requests which are merged here that we want to push to the old repo can be done with some simple git.

@drujensen
Copy link
Member

drujensen commented Apr 2, 2018

@robacarp No, I don't think old builds are broken until we create the "old" repository. They should still work, right?

@robacarp
Copy link
Member

robacarp commented Apr 2, 2018

@drujensen Dang, you're right. It would have. I forgot about the github redirect. I've already created the -orm repo and pushed the latest 0.9.1 code to it, so I'm sure the redirect is dead now.

I've been a little paranoid in this thread because I wasted a day or so a couple weeks ago when the github redirect pointing to Garnet from amber-spec or something died and my app wouldn't install shards anymore. The shard command isn't at all helpful with error messages when a shard it's trying to clone doesn't exist anymore, and I've been trying to avoid anyone going through that trouble as much as possible, so I acted without thinking it through.

The -orm project is archived, and the readme and other parts of the repo point to this one now. If needed, it can be un-archived and tweaked. I'm happy to see to that maintenance.

@robacarp robacarp mentioned this issue Apr 2, 2018
7 tasks
@drujensen
Copy link
Member

This is now complete and released 0.11.0. #204

Framework 2018 automation moved this from To Do to Done May 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

5 participants