Skip to content

Conversation

@alexis-via
Copy link
Contributor

No description provided.

@alexis-via
Copy link
Contributor Author

The problem with double dot in filename is now fixed

@JonathanNEMRY
Copy link

Last refactoring of @lmignon is done here:
akretion#2

For this current PR take also a special attention to akretion#1

-jne

Alexis de Lattre and others added 9 commits December 26, 2016 15:32
The goal is to improve the modularity by making the parser a true inheritable odoo model and share part of the code with the 'report' model

Conflicts:
	report_py3o/models/ir_actions_report_xml.py
	report_py3o/models/py3o_report.py
	report_py3o/tests/test_report_py3o.py
Conflicts:
	report_py3o/models/py3o_report.py
* flake8
@faide
Copy link
Contributor

faide commented Feb 14, 2017

@lmignon is it ok for you to merge this PR as is or do you need more time? I have people who want to base work on yours to port back to older odoo versions...

@lmignon
Copy link
Contributor

lmignon commented Feb 14, 2017

@faide The proposed refactoring akretion#2 works fine in production with a lot of improvements. I test finalize my tests in the next hours of acsone@4048c0b and keep you informed once everything is ok

@lmignon
Copy link
Contributor

lmignon commented Feb 14, 2017

@faide @alexis-via akretion#2 is ready to be merged. I've developed a new module that extends Py3o Report by letting you specify a rule to choose the appropriate template to use according to an expected language....acsone@ac196cc This new module use the changes in the last commit of akretion#2 as extension point.

Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

It's time to move forward 😄

Copy link
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

👍 Code review + tested

Copy link

@JonathanNEMRY JonathanNEMRY left a comment

Choose a reason for hiding this comment

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

Ready for merge...please don't squash commits...

@pedrobaeza
Copy link
Member

Seriously 133 commits without any squash? You have a total crusade against squashing, but I think this is not sustainable as the repo will grow too much in size for no real benefit with several diffs that nullify each other.

@yajo
Copy link
Member

yajo commented Feb 20, 2017

I can see the commit history is huge and mostly irrelevant (many things seem to come from a migration from a mercurial repo, with tagging, updating .hgignore and so on).

Indeed rebasing it would be a nightmare, but also we get lots of unneeded history. There's the fine balance between authorship and usefulness of commits, but in general I'm more against useless commits, so IMHO it would be good if you squash it (at least, per author and following history, we'd still have about 30 commits, but that feels better in the balance).

@alexis-via
Copy link
Contributor Author

I think that 133 commits for a 6-year-long history of work is pretty small. And there are many different authors that worked together on it, so it's not possible to merge all the commits together.

So we should merge this PR WITHOUT SQUASHING anything. I'm not the only one that thinks that this pro-squashing obsession is starting to be unbearable. And there is a significant risk of killing the motivation of contributors like me and others. This thread will be observed by sraps and other Aeroo fans and we are giving them additional arguments NOT to join the OCA... because working in the OCA is starting to be a nightmare and new rules and regulations are invented every month or so.

@pedrobaeza
Copy link
Member

OK, do what you think is correct.

@yajo yajo merged commit dd6d142 into OCA:10.0 Feb 21, 2017
@yajo
Copy link
Member

yajo commented Feb 21, 2017

Ok, merged, not worth it the fight. Thanks for your contribution pals 😉

@adrienpeiffer
Copy link

👏

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.