Skip to content

[12.0][FIX] excel_import_export, use sudo() to get param#1636

Merged
pedrobaeza merged 1 commit into
OCA:12.0from
ecosoft-odoo:12.0-fix-excel_import_export-use_sudo_for_param
Aug 9, 2019
Merged

[12.0][FIX] excel_import_export, use sudo() to get param#1636
pedrobaeza merged 1 commit into
OCA:12.0from
ecosoft-odoo:12.0-fix-excel_import_export-use_sudo_for_param

Conversation

@kittiu

@kittiu kittiu commented Aug 7, 2019

Copy link
Copy Markdown
Member

Small fix. Just to ensure that the param path_temp_file is readable.

@kittiu
kittiu force-pushed the 12.0-fix-excel_import_export-use_sudo_for_param branch from bee009b to f6c33c7 Compare August 7, 2019 09:22
@pedrobaeza pedrobaeza added this to the 12.0 milestone Aug 7, 2019
Comment thread excel_import_export/__manifest__.py Outdated
Comment thread excel_import_export/models/xlsx_export.py Outdated
Comment thread excel_import_export/readme/HISTORY.rst Outdated
@kittiu
kittiu force-pushed the 12.0-fix-excel_import_export-use_sudo_for_param branch from f6c33c7 to 1f801da Compare August 7, 2019 09:54
@kittiu

kittiu commented Aug 7, 2019

Copy link
Copy Markdown
Member Author

@pedrobaeza thank you for the review. Fixed.

@pedrobaeza

Copy link
Copy Markdown
Member

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Rebased to 12.0-ocabot-merge-pr-1636-by-pedrobaeza-bump-patch, awaiting test results.

@pedrobaeza

Copy link
Copy Markdown
Member

I'm seeing a hole here due to changelog: if you don't put version number and this one gets merged after another PR for the same module, the changelog numbering won't be correct (and that's another reason why I don't like static changelog maintenance having commit history).

@sbidoul how do you workaround this problem?

@sbidoul

sbidoul commented Aug 7, 2019

Copy link
Copy Markdown
Member

@pedrobaeza about changelogs yes, there is nice tool named towncrier which basically lets you define changelog entries as small files of the form <issue/pr number>.(bugfix|feature|...). You add such files in the PRs and when releasing towncrier generates the changelog entries (with a nice backlink to the issue/pr) and remove those files.

I'm experimenting it on mis builder, and it is somehting we could at some point consider generalizing, with proper OCA tooling such as bot support.

@pedrobaeza

Copy link
Copy Markdown
Member

OK, that can be the solution then.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-1636-by-pedrobaeza-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.

@pedrobaeza

Copy link
Copy Markdown
Member

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Rebased to 12.0-ocabot-merge-pr-1636-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-1636-by-pedrobaeza-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.

@pedrobaeza

Copy link
Copy Markdown
Member

Not sure why it's failing... let's rety again

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

What a great day to merge this nice PR. Let's do it!
Rebased to 12.0-ocabot-merge-pr-1636-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-1636-by-pedrobaeza-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.

@pedrobaeza

Copy link
Copy Markdown
Member

No clue what's happening. @kittiu can you rebase this PR to see?

@sbidoul

sbidoul commented Aug 7, 2019

Copy link
Copy Markdown
Member

Looks like travis is quite unstable these days. I've restarted the failed travis job.

@kittiu

kittiu commented Aug 9, 2019

Copy link
Copy Markdown
Member Author

@pedrobaez is this ok now to quick merge?
Note: I am working on including the feature from report_excel to excel_import_export.
Thank you!

@pedrobaeza

Copy link
Copy Markdown
Member

Let's merge it manually

@pedrobaeza
pedrobaeza merged commit 0e4479f into OCA:12.0 Aug 9, 2019
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (13.0)
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.

4 participants