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.

Sign up for GitHub

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

Jump to bottom

3.1.4 or 3.1.3 breaks custom operators #521

Closed
Big-Whoop opened this issue Jul 8, 2020 · 4 comments · Fixed by #528
Closed

3.1.4 or 3.1.3 breaks custom operators #521

Big-Whoop opened this issue Jul 8, 2020 · 4 comments · Fixed by #528
Milestone
3.2.0

Comments

@Big-Whoop
Copy link

I just updated pebble from 3.1.2 to 3.1.4 and found that my custom operators no longer work. The BinaryOperator Interface has changed while a patch or minor version normally shouldn't introduce breaking changes on public interfaces.

I also found that the documentation hasn't been updated.

I tried to implement the new interface but now all my operators run into a stack overflow with no obvious reason. I Might be doing something wrong or I discovered a bug.

Is it possible to update the doumentation so I can verify if my implementation is correct?
@ebussieres
Copy link
Member

Here's the pull request that has changed the interface

https://github.com/PebbleTemplates/pebble/pull/499/files

@Big-Whoop
Copy link
Author

Hi, thank you, I found my error. I interpreted the "getInstance()" method so that I should return a singleton instance of the Expression instead of creating a new instance each time. If I do the latter it works as expected.

Maybe createInstance() would be a better and less confusing name for this method because getInstance() is a well known method name for a singleton pattern.

ebussieres added a commit that referenced this issue Jul 25, 2020
@ebussieres
Adjust doc (#521)
4b0f7fd
ebussieres added a commit that referenced this issue Jul 25, 2020
@ebussieres
Adjust doc (#521) (#528)
bf76894
@ebussieres ebussieres linked a pull request Jul 25, 2020 that will close this issue
521 #528
Merged
@ebussieres
Copy link
Member

I've just adjusted the doc. In another major release, I'll rename the getInstance method, so I'll keep it open

@ebussieres ebussieres added this to the 3.2.0 milestone Jul 25, 2020
ebussieres added a commit that referenced this issue Oct 27, 2022
@ebussieres
Rename method getInstance to createInstance in BinaryOperator interfa…
9d7dc0b 
…ce (#521)
@ebussieres ebussieres mentioned this issue Oct 27, 2022
Merged
ebussieres added a commit that referenced this issue Oct 27, 2022
@ebussieres
Rename method getInstance to createInstance in BinaryOperator interfa…
35fc699 
…ce (#521) (#633)
@ebussieres
Copy link
Member

Gonna be available in next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.2.0
Development

Successfully merging a pull request may close this issue.

521 PebbleTemplates/pebble
2 participants
@ebussieres @Big-Whoop