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

Add missing response headers #117

Closed

Conversation

tschuehly
Copy link
Contributor

As discussed in #105 this is the work of @xhaggi rebased on the current main and fixed to align with the current version

source: https://github.com/xhaggi/htmx-spring-boot/tree/missing-response-headers

@tschuehly tschuehly mentioned this pull request Apr 30, 2024
@xhaggi
Copy link
Collaborator

xhaggi commented Apr 30, 2024

I would definitely prefer if you cherry pick the commits so that you have one per annotation and not to lose the author of the commits. @wimdeblauwe or do you have a different opinion on this?

@tschuehly
Copy link
Contributor Author

@xhaggi
Well then I need to solve merge conflicts for each step, it already took two hours as it is now.

/**
* How the response will be swapped in relative to the target
*/
String swap() default "";
Copy link
Owner

Choose a reason for hiding this comment

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

Can't we use HxSwapType here ?

@wimdeblauwe
Copy link
Owner

I would definitely prefer if you cherry pick the commits so that you have one per annotation and not to lose the author of the commits. @wimdeblauwe or do you have a different opinion on this?

I like small commits for sure, but I think this is a nice whole. I don't see much need for separate commits for each annotation. That said, maybe @tschuehly can do a force commit with an added line in the commit message using Co-authored-by (https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors) to acknowledge all the work you have done on it.

@xhaggi
Copy link
Collaborator

xhaggi commented May 1, 2024

Well then I need to solve merge conflicts for each step, it already took two hours as it is now.

@tschuehly I really appreciate your work, but I like to stick to separate commits and the original work I did. That's why I did it on my own and created a separate PR #118 for it. I hope this is OK for you? Maybe you should have asked in #105 if I could do the rebase and a PR, I certainly would have done it. BTW it only took me a few minutes.

@tschuehly
Copy link
Contributor Author

Well now it doesn't matter 😁

@tschuehly tschuehly closed this May 1, 2024
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.

3 participants