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

Pull request for label changes #2

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

betelgeuse
Copy link
Contributor

These were never pulled giving me a chance to try the new github pull request interface.

bryanlarsen and others added 15 commits May 20, 2010 10:03
Hobo patched WillPaginate with hobo_model call. Now the patching
happens with Hobo::Model.enable
Calling hobo_model in a model class body without having called
HoboFields.enable previously results in a SystemStackError with message
stack level too deep. For this reason query if fields method is
available and throw a better error message. [#762 state:resolved]
The included_taglibs parameter of Dryml.render wasn't actually used in
any way. Now it's fixed to be properly passed to Dryml.make_renderer_class
[#761 state:resolved]
We are using WillPaginate::Finder so we should have a require for it.
While writing doctests I stumbled upon it not being defined. Presumably
inside Rails it gets autoloaded in some way.
If you want to use Dryml.render in console you need ActionView and
ActionController. Now the file explicitly requires them so you don't
have to do it yourself before requiring dryml. This also makes it easier
to write doctests.
Previously Rapid did not rander <label> elements inside the forms. This
for example made it impossible to use the labels in cucumber features.
The implementation is not the cleanest around because of two goals:
first be as compatible as possible with the existing implementation and
secondly to avoid calling into Rails internals to figure out the id.
[#759 state:resolved]
@bryanlarsen
Copy link
Collaborator

I like this new pull request interface. I've verified 1,2,3&5 on the system tests. I'm worried about #4 breaking CSS for people, even though it's obviously the right direction to go. I wouldn't mind putting it into 1.1 if we could bundle it up with a few other similar fixups.

With this new github interface, will Matt & Tom see this comment? If so, I'd like an ack from one of them before pushing the 4 I've tested.

As for #6, I'd like an ack from ddnexus.

@bryanlarsen
Copy link
Collaborator

Hmm, it appears the order has changed. afed62e, 978d4ff, 2288c04 and b509f1d have all been pushed into master but do not appear to be in Hobo 1.3. It's certainly possible that all 4 are not relevant for rails3.

The biggest change here is 289d4da. It's the right thing to do, but it will break somebody's CSS, so it cannot go into 1.0, but it should go into 1.1 and 1.3.

bryanlarsen pushed a commit that referenced this pull request Oct 16, 2012
Add activation_email_url route to routes.rb
tjhayasaka pushed a commit to tjhayasaka/hobo that referenced this pull request Sep 30, 2013
@lyrixx
Copy link

lyrixx commented Mar 7, 2017

Could you rebase please ?

@rjmunro
Copy link

rjmunro commented Sep 29, 2017

This is currently the oldest open PR on github... https://github.com/pulls?q=is%3Aopen+is%3Apr+sort%3Acreated-asc

@BoD
Copy link

BoD commented Mar 19, 2021

So any progress on this?

@patrickklaeren
Copy link

Can we get this merged yet?

@PythonCoderAS
Copy link

This is currently the oldest open PR on github... https://github.com/pulls?q=is%3Aopen+is%3Apr+sort%3Acreated-asc

Still is even in 2023 and most likely will stay that way indefinitely

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.

8 participants