-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat(author-jinja): load jinja extensions from plugins #1710
feat(author-jinja): load jinja extensions from plugins #1710
Conversation
Signed-off-by: Ryan Ahearn <ryan.ahearn@gsa.gov>
Signed-off-by: Ryan Ahearn <ryan.ahearn@gsa.gov>
Signed-off-by: Ryan Ahearn <ryan.ahearn@gsa.gov>
Signed-off-by: Ryan Ahearn <ryan.ahearn@gsa.gov>
fix some other random typos Signed-off-by: Ryan Ahearn <ryan.ahearn@gsa.gov>
Signed-off-by: Ryan Ahearn <ryan.ahearn@gsa.gov>
Hi @rahearn - sorry for the delay. I'll look at this in the next few days :). |
Signed-off-by: Ryan Ahearn <ryan.ahearn@gsa.gov>
31834c1
to
011aafb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rahearn Thanks for this submission! Wanted to add some early feedback here. Requesting new source code files (the newly created jinja
package in this case) have the updated copyright info. I added a suggestion on one of the files to what this should be updated to.
Signed-off-by: Ryan Ahearn <ryan.ahearn@gsa.gov>
eaa6246
to
dbe589d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - This is really great work. @jpower432's picked up one comment otherwise the only thing that i've seen here which needs to be tidied here is a dangling comment.
} | ||
|
||
|
||
def discovered_plugins(search_module: str) -> Iterator[Tuple[str, Any]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice improvement :)
trestle/core/jinja/base.py
Outdated
class TrestleJinjaExtension(Extension): | ||
"""Class to define common methods to be inherited from for use in trestle.""" | ||
|
||
# This |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think #this
needs to be cleaned up.
Signed-off-by: Ryan Ahearn <ryan.ahearn@gsa.gov>
518ce69
to
83fed9e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the contribution @rahearn!
@rahearn Your branch needs to be up to date with the base branch and then it should be good to merge. |
Types of changes
develop
->main
)Quality assurance (all should be covered).
Summary
Builds on the proof of concept at #1684 and:
ssp_interface
into a set of jinja filters. Example switch in use:{% for party in ssp_interface.get_parties_for_role(ssp.metadata.responsible_parties, "prepared-by") %}
becomes{% for party in ssp.metadata.responsible_parties | parties_for_role("prepared-by", ssp) %}
{{ ssp_interface.safe_retrieval(address, 'addr_lines', []) | join(' ') }}
becomes{{ address.addr_lines | as_list | join(' ') }}
trestle_{plugin_name}.jinja_ext
Key links:
Before you merge