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

Fragments #91

Merged
merged 16 commits into from
Nov 19, 2024
Merged

Fragments #91

merged 16 commits into from
Nov 19, 2024

Conversation

librasteve
Copy link
Contributor

This request is an implementation of Fragments for the Cro Template language.

It addresses this Feature Request Issue #90 which hopefully provides a rationale for the new feature.

I will shortly make a PR for cro-website to update the docs to include the new feature.

I suggest a point bump in the major version since this extends the API, e.g. to 0.9.0 which I leave to the person merging this PR if it is approved.

@patrickbkr
Copy link
Member

Thanks for putting work into this. This is looking promising. I'll give this a review as soon as I find some time.

Copy link
Member

@patrickbkr patrickbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all looking good. Just some polishing is missing.

README.md Outdated Show resolved Hide resolved
cro-webapp.iml Outdated Show resolved Hide resolved
lib/Cro/WebApp/Template/AST.rakumod Outdated Show resolved Hide resolved
lib/Cro/WebApp/Template/Repository.rakumod Outdated Show resolved Hide resolved
lib/Cro/WebApp/Template/Repository.rakumod Show resolved Hide resolved
lib/Cro/WebApp/Template/AST.rakumod Outdated Show resolved Hide resolved
@librasteve
Copy link
Contributor Author

Good catches in there - thanks!

Hopefully all is now shiny.

I will now go change § to ~ in the docs ;-)

@patrickbkr
Copy link
Member

@librasteve: Are you aware that the Cro project is currently severely lacking maintainers? Currently I and pe_elle (who only recently joined) are the only ones doing maintainer work (currently reviewing is what's needed the most). Given there is a policy of one-review-before-merge, my own PRs are currently effectively frozen because I can't review my own PRs. Would you consider joining the team? It does seem like you know the stack.

@librasteve
Copy link
Contributor Author

yes - I would be very happy to join the team - thank you for the invitation

@patrickbkr
Copy link
Member

I've just sent you an invite. How can you be reached? I guess this github issue is not the best communication platform. :-)

Copy link
Member

@patrickbkr patrickbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

@patrickbkr
Copy link
Member

Are you OK with merging this as well now?

@librasteve
Copy link
Contributor Author

my email is [email protected], also Discord is good

@librasteve
Copy link
Contributor Author

OK - I gather that CI was failing before my PR

@librasteve
Copy link
Contributor Author

Yes, I am happy to proceed with the merge (GH is not giving me a Merge button, so I guess you will have to press the button please)

@patrickbkr patrickbkr merged commit dcaca2a into croservices:master Nov 19, 2024
0 of 4 checks passed
@patrickbkr
Copy link
Member

Yeah, I know what's wrong with the CI. Fix is upcoming.

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.

2 participants