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

[14.0][FIX] base_delivery_carrier_files: Reset cursor back for StringIO() #612

Merged

Conversation

Kiplangatdan
Copy link
Member

No description provided.

@Kiplangatdan Kiplangatdan marked this pull request as draft February 21, 2023 17:48
@Kiplangatdan Kiplangatdan marked this pull request as ready for review February 21, 2023 17:49
@Kiplangatdan
Copy link
Member Author

@thomaspaulb This fixes the binary data when you have multiple rows

@thomaspaulb
Copy link

To be more complete, for other reviewers to understand:

Problem was: The lines in the generated carrier file had a bunch of leading ASCII 0x00 characters, the amount of which was increasing for each subsequent line. This caused the file to become seen as binary instead of text, plus have a lot of useless content.

@thomaspaulb
Copy link

@ntsirintanis @NL66278 Mind reviewing also

@rousseldenis
Copy link
Contributor

@Kiplangatdan Maybe a test that highlight this change ?

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@github-actions
Copy link

github-actions bot commented Jul 2, 2023

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 2, 2023
@thomaspaulb
Copy link

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Sorry @thomaspaulb you are not allowed to merge.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@thomaspaulb
Copy link

@OCA/logistics-maintainers Could this be merged?

@jbaudoux
Copy link
Contributor

jbaudoux commented Jul 2, 2023

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-612-by-jbaudoux-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jul 2, 2023
Signed-off-by jbaudoux
@OCA-git-bot
Copy link
Contributor

@jbaudoux your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-612-by-jbaudoux-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 9, 2023
@rousseldenis
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-612-by-rousseldenis-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit ccf7f34 into OCA:14.0 Jul 10, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 8d151b1. Thanks a lot for contributing to OCA. ❤️

@thomaspaulb thomaspaulb deleted the 14.0-fix-base-delivery-carrier-files branch July 10, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants