First Commit Scrum Project Module #1

mohamedhabibchallouf · 2017-11-23T12:59:15Z

Add Scrum module to project agile Branch

elicoidal

Hi @mohamedhabibchallouf
Thanks for this first commit!

Please check https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md for the guidelines (and directories for templates).

Please fix travis and make it "green".

No test yet done on my side 😄

elicoidal · 2017-11-24T00:40:57Z

.settings/org.eclipse.core.resources.prefs

@@ -0,0 +1,4 @@
+eclipse.preferences.version=1


You should remove this directory (add it in .gitignore)

elicoidal · 2017-11-24T00:41:34Z

project_scrum/__init__.py

@@ -0,0 +1,3 @@
+# -*- coding: utf-8 -*-


Please add a README file following: https://github.com/OCA/maintainer-tools/blob/master/template/module/README.rst

elicoidal · 2017-11-24T00:41:46Z

project_scrum/__manifest__.py

+
+{
+    'name': 'Project Scrum',
+   'summary': 'Use Scrum Method  to manager your project',


remove extra space

elicoidal · 2017-11-24T00:42:13Z

project_scrum/__manifest__.py

+{
+    'name': 'Project Scrum',
+   'summary': 'Use Scrum Method  to manager your project',
+    'version': '10',


version 10.0.1.0.0 (following: https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#version-numbers)

elicoidal · 2017-11-24T00:42:32Z

project_scrum/__manifest__.py

+   'summary': 'Use Scrum Method  to manager your project',
+    'version': '10',
+    'category': 'Project Management',
+    'description': """


This key is not needed in v10 (README file instead)

elicoidal · 2017-11-24T00:57:39Z

project_scrum/sequences_projects.xml

+<openerp>
+	<data noupdate="0">
+
+


remove extra lines

elicoidal · 2017-11-24T00:58:34Z

project_scrum/sequences_projects.xml

+		<record id="seq_use_story" model="ir.sequence">
+			<field name="name">Story Number</field>
+			<field name="code">user.story</field>
+			<field name="prefix">N°:%(year)s/%(month)s:</field>


remove "N°:" and final ":"

elicoidal · 2017-11-24T00:59:00Z

project_scrum/static/description/index.html

+          <div class="item oe_img_bg active" style="background-image: url(&quot;http://localhost:8069/website/static/src/img/banner/mango.jpg&quot;); height:300px;">
+            <div class="container">
+              <div class="row content">
+                <!--<h1 class="carousel-content col-sm-12 mb16 mt128 col-md-3 col-md-offset-1">Planning Work With Scrum</h1>-->


remove dead codes

elicoidal · 2017-11-24T01:00:17Z

project_scrum/wizard/project_scrum_test_task_view.xml

+    <data>        
+        <record model="ir.values" id="ps_make_test_task">
+            <field name="model_id" ref="model_project_scrum_sprint"/>
+            <field name="name">Creat tasks for test</field>


elicoidal · 2017-11-24T01:00:24Z

project_scrum/wizard/project_scrum_test_task_view.xml

+        </record>
+
+        <record model="ir.actions.server" id="ps_action_server_creat_test_task">
+            <field name="name">Creat Tasks from Test Cases</field>


elicoidal · 2017-11-24T01:13:32Z

Actually it seems that the repo setup files have not been initiated.
Can you create a PR to init the repo files, following example on https://github.com/OCA/business-requirement (for example). We are talking about:

README.md
travis.yml
LICENSE file
CONTRIBUTING.md
oca_dependencies.txt

bealdav · 2017-11-24T07:17:59Z

project_scrum/project_scrum.py

+        else:
+            pass
+
+class project_user_stories(models.Model):


Class name must be camel case. Check class below too

Thanks

bealdav · 2017-11-24T07:21:31Z

project_scrum/project_scrum.py

+            return [], None
+
+class project_actors(models.Model):
+    _name = 'project.scrum.actors'


Please follow Odoo conventions instead of exceptions.
Each record of this model is a record of project.scrum.actor kind/type
Table is a collection of record if this type

bealdav · 2017-11-24T07:45:42Z

@mohamedhabibchallouf thanks a lot for this super module

hbrunn

thanks! please don't get frustrated by the many nags, you only get so many because people are happy about your work and review closely.

@pedrobaeza @dreispt do you still need to configure CI? I see neither runbot nor travis. When travis has run, it will have a lot to complain too, so you could have a look at our coding conventions anyways: https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md

hbrunn · 2017-11-25T14:14:20Z

project_scrum/project_scrum.py

+
+_logger = logging.getLogger(__name__)
+
+class scrum_sprint(models.Model):


use CamelCase for class names, one file per model, and they live in /models

hbrunn · 2017-11-25T14:15:46Z

project_scrum/project_scrum.py

+            ])
+        if len(sprint) > 0:
+            return sprint[0]
+        return None


I'd suggest to return an empty browse record here - then code using this function can treat the result with less caution

hbrunn · 2017-11-25T14:16:50Z

project_scrum/project_scrum.py

+            ('project_id', '=', project_id)
+            ])
+        if len(sprint) > 0:
+            return sprint[0]


why make assumptions? if you really only want to return the first, add a limit clause above. and combine with the below, you can simply return the result of search

hbrunn · 2017-11-25T14:18:38Z

project_scrum/project_scrum.py

+        if self.project_id and self.project_id.manhours:
+            self.planned_hours = self.project_id.manhours
+        else:
+            self.planned_hours = 0.0


given the way empty browse records behave, all this is equivalent to self.planned_hours = self.project_id.manhours

hbrunn · 2017-11-25T14:19:37Z

project_scrum/project_scrum.py

+                self.date_stop = fields.Date.from_string(self.date_start) +\
+                 timedelta(days=self.project_id.default_sprintduration)
+        else:
+            pass


this does nothing. it's also better when you don't do anything in an else branch, to better check for the inverse and exit early. would save you two levels of indentation here

hbrunn · 2017-11-25T14:36:57Z

project_scrum/project_scrum.py

+    user_story_ids = fields.One2many(comodel_name="project.scrum.us", inverse_name="project_id", string="User Sories")
+    meeting_ids = fields.One2many(comodel_name="project.scrum.meeting", inverse_name="project_id", string="Meetings")
+    test_case_ids = fields.One2many(comodel_name="project.scrum.test", inverse_name="project_id", string="Test Cases")  
+    sprint_count = fields.Integer(compute='_sprint_count', string="Sprints")


store all of them, those fields tend to be major performance eaters on bigger installations

hbrunn · 2017-11-25T14:38:36Z

project_scrum/project_scrum.py

+                                            help="Default Sprint time for this project, in days")
+    manhours = fields.Integer(string='Man Hours', required=False,
+                               help="How many hours you expect this project needs before it's finished")
+    description = fields.Text()


not better Html()?

hbrunn · 2017-11-25T14:38:53Z

project_scrum/project_scrum.py

+    _order = 'sequence_test'
+
+    name = fields.Char(string='Name', required=True)
+    project_id = fields.Many2one(comodel_name='project.project', string='Project', ondelete='set null', select=True, track_visibility='onchange', change_default=True)


hbrunn · 2017-11-25T14:40:34Z

project_scrum/project_scrum.py

+    sequence_test = fields.Integer(string='Sequence', select=True)
+    stats_test = fields.Selection([('draft', 'Draft'), ('in progress', 'In Progress'), ('cancel', 'Cancelled')], string='State', required=False)
+    company_id = fields.Many2one(related='project_id.analytic_account_id.company_id')
+    color = fields.Integer(related='project_id.color')


this model might also profit from the kanban stage mixin

hbrunn · 2017-11-25T14:41:08Z

project_scrum/project_scrum.py

+    company_id = fields.Many2one(related='project_id.analytic_account_id.company_id')
+    color = fields.Integer(related='project_id.color')
+
+    def _resolve_project_id_from_context(self, cr, uid, context=None):


also v8 api. consider moving the project related functionality to a mixin anyways

pedrobaeza · 2017-11-26T01:20:17Z

@hbrunn I don't know, as .travis.yml seems to be in place.

hbrunn · 2017-11-26T07:12:20Z

a user with admin rights needs to go to https://travis-ci.org/OCA/project-agile and turn it on, I don't have the permissions

pedrobaeza · 2017-11-26T12:23:44Z

Ouch, it was that! @dreispt, please check next time full "Create new PSC project" procedure, that includes this.

Done. Closing and reopening for triggering Travis.

hbrunn · 2017-11-26T16:20:51Z

@mohamedhabibchallouf I'm not such a fan of automated tools myself, so I never tried it, but there's https://github.com/OCA/maintainer-tools/blob/master/tools/autopep8_extended.py

mohamedhabibchallouf · 2017-11-26T16:26:10Z

@hbrunn i work on it now :)

pnajman-modoolar · 2017-11-27T13:01:54Z

Hi guys,

I'm really happy to see the first pull request here and I appreciate the effort.

In order to prevent any misunderstanding I would like to clarify that this pull request is not related, in any way, to the project_agile (a.k.a Odoo Scrummer) code which will be released here in the up coming days.

Here at Modoolar we are working to make modules compliant with the OCA coding standards and once we finish, we'll start making pull requests for each module (one by one) just as we agreed upon in the "Odoo Scrummer" mail thread.

Cheers,
Petar Najman

hbrunn · 2017-11-27T13:17:05Z

but then this PR is the good place to agree on a data model for the basic entities. We can't have two scrum related modules, and first PR merged wins...

dreispt · 2017-11-27T13:22:31Z

@pedrobaeza Sure, just remember to share it with me.The doc I could find does not mention TravisCI.

@pnajman-modoolar @mohamedhabibchallouf I can see that we have two independently implemented scrum modules. There is no point in having two modules doing the same. We need to check what each does differently and on what they overlap.

pedrobaeza · 2017-11-27T13:28:29Z

@dreispt it's an internal OCA board task (https://odoo-community.org/web?#id=264&view_type=form&model=project.task&action=468&active_id=2), as it contains sensible information.

hbrunn · 2017-11-27T13:32:37Z

...and if it turns out that they have interesting but incongruent features, we should try hard to move as much as possible to a shared base module so that the whole set of modules stays compatible

bealdav · 2017-11-27T13:48:08Z

@pnajman-modoolar there was a real confusion here. Thanks to notice us.
@mohamedhabibchallouf which additionnal feature brings your module ?

Thanks for your work

dreispt · 2017-11-27T17:44:59Z

I added the "Question" tag to remind us to compare with the Scrummer module, to be made available soon, before this can be merged.

dreispt · 2017-11-27T17:46:38Z

@elicoidal The dotfiles are there; the branch needs a rebase to have them included.

hbrunn · 2017-11-28T09:33:29Z

@mohamedhabibchallouf @HaojunZou @anderswallenquist In this PR, I identified quite striking similarities with the vertelab scrum version

Is this an innocent case of code borrowing/using some common ancestor and forgetting to attribute or is this a case of license violation?

@mohamedhabibchallouf please comment on this. In any case, you must be sure that the code you hand in is actually yours to give away. The other source being some free license is not enough, because by submitting code to the OCA you also push it under the OCA's CLA, which is an extra legal step.

mohamedhabibchallouf · 2017-11-28T10:40:41Z

@hbrunn
Actually In Tenovar Ltd we are working on this module, since an early version (OpenERP 7), now I am pushing the code in his latest version (Odoo v10),
I just discovered that there is a similarity with vertlab, In any case, I can confess, that the code I am pushing is a result of tenovar working, and its a free to use, we are pushing it because we believe in the Opensource and sharing knowledge values.

hbrunn · 2017-11-28T10:59:48Z

good to hear. I just want to avoid merging time bombs in the sense that if other people have a claim to code we in the OCA use, this will be a problem sooner or later. Can you trace it in your organization internally where the code originally comes from?

hbrunn · 2017-11-28T11:21:54Z

@pedrobaeza does the secret list also say something about runbot? Would be good I think to have this in place for the functional people to discuss the functional aspects of this

pedrobaeza · 2017-11-28T12:35:43Z

@hbrunn yes, the procedure includes 2 steps for runbot:

Create a webhook in GitHub repository with runbot URL.
Add the repository in our runbot instance.

@dreispt did you do both?

dreispt · 2017-11-28T14:05:56Z

@mohamedhabibchallouf Hmm, your review and retrospective field definitions are identical to Vertelab's, introduced in a 2015 commit...

Is there a chance someone borrowed this code?
It is AGPL open source, so of course you can use it. However, authorship attribution is required by the license, otherwise it would be plagiarism.
And the OCA has an additional requirement that the authors need to have signed the CLA.
If this is so, we should add vertelab to the authors and ask them to agree with the CLA.

What do you think @mohamedhabibchallouf ?

samirGuesmi · 2017-11-28T14:31:48Z

@dreispt
Hi, I would thank everyone here for the effort and checking,
Here we should mention that the base Module, was done by Odoo SA, (OpenERP V6). Vertelab and others propably based on that module and start putting more functionalities and trying to cater for more Agile-Scrum values.
As a Tenovar Ltd we did an effort on functional and technical side, Saying that does not mean that we are against adding more authors, we pushed it into OCA community to share our experience around Agile Scrum methodolgy.
Contributors are more than welcome, and of course, will be added to the authors.

hbrunn · 2017-11-28T14:42:40Z

that this all derived from odoo's project_scrum somehow is clear and nobody questions this. That's also not the problem we're talking about here. The question is: Is this a further development of only odoo's sources or is this a derived work of @vertelab's derived work, which would force us to ask them for waiving their copyright or signing our CLA if we want to use the code. Alternative is to simply rewrite sections of code in question, preferably by a person who hasn't seen the original code before.

I pinged two of the authors involved in the other module in #1 (comment), maybe they can shed light into this.

The similarity I've found was created long after Odoo SA deprecated the module, so to be honest my assumption is you grabbed this code some time in the 8 lifecycle and kept improving it. Absolutely no problem with that in the open source world, but unfortunately, that still doesn't make it yours in terms of license decisions. Please clarify.

samirGuesmi · 2017-11-28T15:16:35Z

@hbrunn Thanks, I will try to check the history of our Module.

Otherwise, following your logic, we need to ask Odoo SA , as well as the Python Software Foundation since we are using python for coding.

Again, our only intention is to share, an opensource, done by us from scratch or improved by us, to the community.

robch2342 · 2017-11-28T15:34:41Z

@hbrunn
For what it's worth, I have signed the ICLA, and I believe my boss (Anders Wallenquist) signed the CCLA for Vertel AB as well. If this is a derivative of our work then we would ofc want Vertel AB to be mentioned among the authors.

hbrunn · 2017-11-28T15:53:19Z

now, that's great to hear. Then we'll be fine one way or the other (@dreispt can you confirm the CCLA? I don't know if a humble OCA member can see this at all, what I know is that I don't know how), and I leave it to you to gather the evidence for the authorship. But then we're sure we don't pull in code of people who don't want it to be pulled in, that was my whole point. Thanks.

So back to technical stuff!

dreispt · 2017-11-28T16:22:19Z

I checked and Vertel AB has an ECLA, signed by CEO Anders Wallenquist.

dreispt · 2017-11-29T11:02:39Z

project_scrum/project_scrum.py

@@ -251,7 +256,8 @@ def _resolve_project_id_from_context(self, cr, uid, context = None):
            return context['default_project_id']
        if isinstance(context.get('default_project_id'), basestring):
            project_name = context['default_project_id']
-            project_ids = self.pool.get('project.project').name_search(cr, uid, name=project_name, context=context)
+            project_ids = self.pool.get('project.project').name_search(cr, uid, 
+                                                                       name=project_name, context=context)


I personally dislike this formatting style. in case you agree with me, you can try breaking the line in a more space efficient way, like this:

project_ids = self.pool.get('project.project').name_search( cr, uid, name=project_name, context=context)

dreispt · 2017-11-30T17:25:41Z

project_scrum/project_scrum.py

-    stats_test = fields.Selection([('draft', 'Draft'), ('in progress', 'In Progress'), ('cancel', 'Cancelled')], string='State', required=False)
+    stats_test = fields.Selection([('draft', 'Draft'),
+                                   ('in progress', 'In Progress'),
+                                   ('cancel', 'Cancelled')], string='State', required=False)


To be PEP8 compliant, the line break must be right after the (, so that the cr, uid, go at the beginning of the next line.
Anyway, the cr, uid, should go away soon, when the port to the new API.

daramousk · 2018-05-16T11:26:44Z

@mohamedhabibchallouf I have made a PR that fixes a lot of stuff (this module is seriously broken). Are you going to follow up on this PR or should I start a new one?

hbrunn · 2018-05-16T11:33:29Z

@daramousk please start a new one, this PR hasn't been updated for more than half a year

hbrunn · 2018-05-17T18:43:51Z

closing this in favor of #16 because of lack of activity and because we commit to get this done

First Commit Scrum Project Module

694fce4

elicoidal suggested changes Nov 24, 2017

View reviewed changes

elicoidal added the needs fixing label Nov 24, 2017

elicoidal added this to the 10.0 milestone Nov 24, 2017

bealdav reviewed Nov 24, 2017

View reviewed changes

hbrunn requested changes Nov 25, 2017

View reviewed changes

pedrobaeza closed this Nov 26, 2017

pedrobaeza reopened this Nov 26, 2017

dreispt added the question label Nov 27, 2017

--Fix Code/Views scrum project with maintainer-tools OCA

7f417ae

remove org.eclipse.core.ressource

97dc8ee

dreispt reviewed Nov 29, 2017

View reviewed changes

-m "update format python code"

2af1477

dreispt reviewed Nov 30, 2017

View reviewed changes

rafaelbn mentioned this pull request Apr 4, 2018

odoo-gdpr to OCA? vertelab/odoo-gdpr#1

Closed

daramousk mentioned this pull request May 17, 2018

[WIP][MIG] Migration of project_scrum to 10.0 #16

Merged

hbrunn closed this May 17, 2018


		_logger = logging.getLogger(__name__)

		class scrum_sprint(models.Model):

First Commit Scrum Project Module #1

First Commit Scrum Project Module #1

Conversation

mohamedhabibchallouf commented Nov 23, 2017

elicoidal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elicoidal commented Nov 24, 2017

bealdav Nov 24, 2017 • edited Loading

Choose a reason for hiding this comment

bealdav Nov 24, 2017 • edited Loading

Choose a reason for hiding this comment

bealdav commented Nov 24, 2017

hbrunn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedrobaeza commented Nov 26, 2017

hbrunn commented Nov 26, 2017 • edited Loading

pedrobaeza commented Nov 26, 2017

hbrunn commented Nov 26, 2017

mohamedhabibchallouf commented Nov 26, 2017

pnajman-modoolar commented Nov 27, 2017

hbrunn commented Nov 27, 2017

dreispt commented Nov 27, 2017

pedrobaeza commented Nov 27, 2017

hbrunn commented Nov 27, 2017

bealdav commented Nov 27, 2017 • edited Loading

dreispt commented Nov 27, 2017

dreispt commented Nov 27, 2017

hbrunn commented Nov 28, 2017

mohamedhabibchallouf commented Nov 28, 2017

hbrunn commented Nov 28, 2017

hbrunn commented Nov 28, 2017

pedrobaeza commented Nov 28, 2017

dreispt commented Nov 28, 2017

samirGuesmi commented Nov 28, 2017

hbrunn commented Nov 28, 2017

samirGuesmi commented Nov 28, 2017

robch2342 commented Nov 28, 2017 • edited Loading

hbrunn commented Nov 28, 2017

dreispt commented Nov 28, 2017

Choose a reason for hiding this comment

dreispt Nov 30, 2017 • edited Loading

Choose a reason for hiding this comment

daramousk commented May 16, 2018

hbrunn commented May 16, 2018

hbrunn commented May 17, 2018

bealdav Nov 24, 2017 •

edited

Loading

bealdav Nov 24, 2017 •

edited

Loading

hbrunn commented Nov 26, 2017 •

edited

Loading

bealdav commented Nov 27, 2017 •

edited

Loading

robch2342 commented Nov 28, 2017 •

edited

Loading

dreispt Nov 30, 2017 •

edited

Loading