Active Resource should warn about missing prefix value #37

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
2 participants
@gramos
Contributor

gramos commented Sep 20, 2010

Hi, I been working on this:
https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/5631-activeresourcemissingprefixparam-proposal

but I figured out that the last changes on activeresource maybe will broke my
patch, so I forked rails and I did my commits on my fork on master and 3-0-stable, I'm going to apply my changes for 2.3 soon I will send it a new pull request.

Thanks in advance.

Gastón Ramos

@mikel

This comment has been minimized.

Show comment
Hide comment
@mikel

mikel Sep 22, 2010

Member

Hey, nice work.

I'll pull in and test it out today.

Member

mikel commented Sep 22, 2010

Hey, nice work.

I'll pull in and test it out today.

@mikel

This comment has been minimized.

Show comment
Hide comment
@mikel

mikel Sep 22, 2010

Member

Ideally, your pull request wouldn't have a merge commit in there. Please in the future rebase your work. Makes life easier for me in committing.

Member

mikel commented Sep 22, 2010

Ideally, your pull request wouldn't have a merge commit in there. Please in the future rebase your work. Makes life easier for me in committing.

@gramos

This comment has been minimized.

Show comment
Hide comment
@gramos

gramos Sep 22, 2010

Contributor

Thanks for the advice!

Contributor

gramos commented Sep 22, 2010

Thanks for the advice!

@gramos

This comment has been minimized.

Show comment
Hide comment
@gramos

gramos Sep 24, 2010

Contributor

Do you need that I remove the merge commit and do the pull request again?

Contributor

gramos commented Sep 24, 2010

Do you need that I remove the merge commit and do the pull request again?

@mikel

This comment has been minimized.

Show comment
Hide comment
@mikel

mikel Sep 27, 2010

Member

Hi there Gramos,

I totally forgot about this.

If you could rebase against master without the merge, and do another pull request, that would be fantastic. I'll take a look at it tomorrow morning and see how we go.

As for associations, yes, good idea. For that I would request you post your idea to rails-core and have a discussion on the implementation first.

Thanks for your work, look forward to your pull request tomorrow morning my time :)

Mikel

Member

mikel commented Sep 27, 2010

Hi there Gramos,

I totally forgot about this.

If you could rebase against master without the merge, and do another pull request, that would be fantastic. I'll take a look at it tomorrow morning and see how we go.

As for associations, yes, good idea. For that I would request you post your idea to rails-core and have a discussion on the implementation first.

Thanks for your work, look forward to your pull request tomorrow morning my time :)

Mikel

seuros pushed a commit to seuros/django that referenced this pull request Aug 12, 2014

claudiob pushed a commit to claudiob/rails that referenced this pull request Sep 23, 2014

claudiob pushed a commit to claudiob/rails that referenced this pull request Sep 23, 2014

dhh added a commit to dhh/rails that referenced this pull request Dec 14, 2015

Merge pull request #37 from rails/downgrade-celluloid
Use Celluloid 0.16.0 until termination issue in 0.17.0 is resolved

kangkyu pushed a commit to kangkyu/rails that referenced this pull request Jul 3, 2016

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment