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

LPS-65325 changes to horizontal and vertical theme #3977

Closed
wants to merge 15 commits into from

Conversation

juliocamarero
Copy link

No description provided.

@juliocamarero juliocamarero changed the title LPS-65325 autogenerated LPS-65325 changes to horizontal and vertical theme Apr 26, 2016
@juliocamarero
Copy link
Author

cc @marcoscv-work

@juliocamarero
Copy link
Author

@drewbrokke @pei-jung I have moved the logic from the themes about users to a theme context contributor in the users-admin-web module.

Can you please have a look?

thanks!

@natecavanaugh
Copy link
Owner

@juliocamarero With moving the user variables out of the unstyled theme, doesn't this technically break backwards compatibility, since any theme that's using those vars will now break?
Or were they only recently added?
Either way, I think we'll have to bump the version on it won't we?

Thanks guys,

@juliocamarero
Copy link
Author

Hey Nate,

Nothing changes for other themes. Now the same variables are injected
before the execution of init.ftl in the theme.

This just makes the themes lighter and the execution faster, but it's a
transparent change for developers.
El El mar, 26 abr 2016 a las 19:02, Nate Cavanaugh notifications@github.com
escribió:

@juliocamarero https://github.com/juliocamarero With moving the user
variables out of the unstyled theme, doesn't this technically break
backwards compatibility, since any theme that's using those vars will now
break?
Or were they only recently added?
Either way, I think we'll have to bump the version on it won't we?

Thanks guys,


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3977 (comment)

@natecavanaugh
Copy link
Owner

@juliocamarero Thanks for the info. Makes sense :)

@pei-jung I believe 065fbd9 already has those removed? Unless I misunderstood what you meant.

@pei-jung
Copy link

Hey @natecavanaugh,

Those variables are only removed in the freemarker template init.ftl but not the velocity template init.vm. I'm not sure if the context contributor will inject the variables to both templates. That's why I wonder if the variables in the velocity should be removed or not.

@natecavanaugh
Copy link
Owner

Ahhh sorry, I misread what you pasted. I'll leave that one to Julio to answer :)

@pei-jung
Copy link

@natecavanaugh That's alright. I should've just linked the url instead of pasting the whole thing. That way, it's more clear :)

@juliocamarero
Copy link
Author

Yes, you are right @pei-jung !! Sorry I missed that one! I forgot we still have the vm files.

I just added another commit.

thanks a lot!!

@natecavanaugh
Copy link
Owner

Merged and pushed to Brian. Thanks :)
Pull request submitted at: brianchandotcom#39130
View total diff: 56a88d8...3e12641

@drewbrokke
Copy link

@juliocamarero thanks for letting us know about this.

Good catch, @pei-jung!

@juliocamarero juliocamarero deleted the pr-4135 branch August 13, 2018 11:39
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

5 participants