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

Delta request-merge #97

Merged
merged 6 commits into from
Jan 24, 2019
Merged

Delta request-merge #97

merged 6 commits into from
Jan 24, 2019

Conversation

ayush1999
Copy link
Contributor

@ayush1999 ayush1999 commented Jan 22, 2019

Changes proposed: A new bug_extractor, that returns the time difference between request date and merge date.

Example:

>>> a = delta_request_merge()
>>> a(bug)
('55.0', datetime.datetime(2017, 8, 8, 7, 0, tzinfo=<UTC>))

If I'm not wrong, 55 is the required delta here. (right?)
@marco-c Is this similar to what was required?

@@ -131,6 +134,12 @@ def __call__(self, bug):
return any(bug['creator_detail']['email'].endswith(domain) for domain in ['@mozilla.com', '@mozilla.org'])


class delta_request_merge(object):
def __call__(self, bug):
uplift_request_datetime = datetime.strptime(bug['history'][0]['when'], '%Y-%m-%dT%H:%M:%SZ').replace(tzinfo=timezone.utc)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the date of the first modification to the bug, we should get the date of the first uplift request instead.

@marco-c
Copy link
Collaborator

marco-c commented Jan 22, 2019

If I'm not wrong, 55 is the required delta here. (right?)

55 is the version of Firefox.

You should calculate the difference between the uplift request date and the datetime.datetime(2017, 8, 8, 7, 0, tzinfo=<UTC>)

@ayush1999
Copy link
Contributor Author

ayush1999 commented Jan 22, 2019

For the first uplift request, I'd have to iterate through bug['history'] to search for an added key starting from approval-mozilla right? (And return the corresponding date).

@marco-c
Copy link
Collaborator

marco-c commented Jan 22, 2019

Exactly, look at the uplift model for an example.

@ayush1999
Copy link
Contributor Author

Yep, that's where I saw this. After these changes, we get datetime.timedelta(26, 40819) as a result for an arbitrary example. (26 days and 40819 seconds). In which format should this be standardized? (Something like 26.X days?)

@marco-c
Copy link
Collaborator

marco-c commented Jan 24, 2019

Yes, I think days would be good.

@ayush1999
Copy link
Contributor Author

ayush1999 commented Jan 24, 2019

Ok, everything gets converted to days now (I'm not sure why. but the travis test takes forever). Also, @marco-c small question: I just noticed that https://github.com/mozilla/bugbug/blob/master/bugbug/bug_features.py#L135 uses the commits key in the bug, which isn't available in the bugs data (so I'm guessing this isn't a regular feature extractor.) . Can you tell me how is this supposed to work?

@marco-c
Copy link
Collaborator

marco-c commented Jan 24, 2019

Can you tell me how is this supposed to work?

The commit data is added to the bugs if you use commit_data=True:

if self.commit_map is not None:
.

@@ -132,6 +135,18 @@ def __call__(self, bug):
return any(bug['creator_detail']['email'].endswith(domain) for domain in ['@mozilla.com', '@mozilla.org'])


class delta_request_merge(object):
def __call__(self, bug):
for change in bug['history']:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Call this history and ind_change change, to keep the same variable names as other parts of the project.

Copy link
Collaborator

@marco-c marco-c left a comment

Choose a reason for hiding this comment

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

Thanks!

@ayush1999
Copy link
Contributor Author

Travis CI fails due to an error in test_cleanup_synonyms. This can be merged now; that test can be fixed in a separate PR.

@marco-c
Copy link
Collaborator

marco-c commented Jan 24, 2019

Travis CI fails due to an error in test_cleanup_synonyms. This can be merged now; that test can be fixed in a separate PR.

The error should be gone if you rebase on top of current master.

@marco-c marco-c merged commit 1331682 into mozilla:master Jan 24, 2019
@ayush1999 ayush1999 deleted the dev branch January 24, 2019 18:02
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.

2 participants