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

refactor: modularize xmlupload (DEV-2813) #568

Merged
merged 5 commits into from Oct 14, 2023
Merged

Conversation

BalduinLandolt
Copy link
Collaborator

No description provided.

@BalduinLandolt BalduinLandolt changed the title refactor: modularize xmlupload refactor: modularize xmlupload (DEV-2813) Oct 13, 2023
@linear
Copy link

linear bot commented Oct 13, 2023

@BalduinLandolt BalduinLandolt marked this pull request as ready for review October 13, 2023 09:45
Copy link
Collaborator

@Nora-Olivia-Ammann Nora-Olivia-Ammann left a comment

Choose a reason for hiding this comment

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

I haven't looked at it in extreme detail but it looks good to me. Please tell me if you would like a closer inspection.

@BalduinLandolt
Copy link
Collaborator Author

I haven't looked at it in extreme detail but it looks good to me. Please tell me if you would like a closer inspection.

I'm pretty confident that I haven't broken anything (says he, right after breaking something and having to fix it...). The most important thing to me would be to know if the steps that are now being done in the big xmlupload function, are logical and reasonably well divided into sub-steps

@Nora-Olivia-Ammann
Copy link
Collaborator

I haven't looked at it in extreme detail but it looks good to me. Please tell me if you would like a closer inspection.

I'm pretty confident that I haven't broken anything (says he, right after breaking something and having to fix it...). The most important thing to me would be to know if the steps that are now being done in the big xmlupload function, are logical and reasonably well divided into sub-steps

I will play it trough in the code this afternoon.

@BalduinLandolt
Copy link
Collaborator Author

I haven't looked at it in extreme detail but it looks good to me. Please tell me if you would like a closer inspection.

I'm pretty confident that I haven't broken anything (says he, right after breaking something and having to fix it...). The most important thing to me would be to know if the steps that are now being done in the big xmlupload function, are logical and reasonably well divided into sub-steps

I will play it trough in the code this afternoon.

I'll merge it as it is for now, but we can still improve on it, if there is something you don't like. Just let me know!

@BalduinLandolt BalduinLandolt enabled auto-merge (squash) October 14, 2023 09:34
@BalduinLandolt BalduinLandolt merged commit 8026fcb into main Oct 14, 2023
6 checks passed
@BalduinLandolt BalduinLandolt deleted the wip/improve-xmlupload branch October 14, 2023 09:39
@daschbot daschbot mentioned this pull request Oct 14, 2023
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

3 participants