Skip to content
This repository has been archived by the owner on Dec 4, 2017. It is now read-only.

Bug 1050131 Write a script to auto-extract from mozilla-central the standalone content into the loop-client git repo #38

Merged
merged 1 commit into from
Aug 12, 2014

Conversation

Standard8
Copy link
Member

No description provided.

import os
from datetime import datetime
import subprocess
import pytz, dateutil, dateutil.tz
Copy link
Contributor

Choose a reason for hiding this comment

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

If you import dateutil you don't need to import dateutil.tz.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, in testing, it appears that both are needed to be imported, so I've added it back again.

@Natim
Copy link
Contributor

Natim commented Aug 11, 2014

nit: In python we usually don't use camelCase for function names. But it is not a big deal.

# to avoid attempting to port the same cset all the time
lastCset = cset

if "merge" in cset.description().lower():
Copy link
Member

Choose a reason for hiding this comment

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

Will this get tripped if somebody happens to use the word "merge" in a checkin comment? If so, perhaps we can find a more robust way to indicate this...

Copy link
Contributor

Choose a reason for hiding this comment

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

An easy way is to count the number of parents. If there is more than one it is a merge commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, fixed.

@dmose
Copy link
Member

dmose commented Aug 11, 2014

Just a few general thoughts:

About the hg reverse mapping:
** does that overwrite or append/prepend to existing commentary?
** if the latter, is it easily extractable in an automated fashion?
** if the former, are we ok losing the old data?

In some ideal world, it would either append or prepend and be easily extractible for the future.

Also, you might want to change the verbiage from "translate" to "update paths" or something, as this will make it more clear to folks running and reading the script what's going on.

You might also run flake8 (the thing we used to lint Talkilla python stuff) before landing so that we have a single coherent style in the script.

@Standard8
Copy link
Member Author

Just a few general thoughts:

About the hg reverse mapping:
** does that overwrite or append/prepend to existing commentary?
** if the latter, is it easily extractable in an automated fashion?
** if the former, are we ok losing the old data?

In some ideal world, it would either append or prepend and be easily extractible for the future.

As discussed on irc, it appends with \nmozilla-central hg revision: <cset>. This should be good enough.

Also, you might want to change the verbiage from "translate" to "update paths" or something, as this will make it more clear to folks running and reading the script what's going on.

Fixed

You might also run flake8 (the thing we used to lint Talkilla python stuff) before landing so that we have a single coherent style in the script.

Done and fixed to the standard flake8 defaults.

…tandalone content into the loop-client git repo. r=natim,dmose
@Standard8
Copy link
Member Author

All comments addressed, merging.

Standard8 added a commit that referenced this pull request Aug 12, 2014
Bug 1050131 Write a script to auto-extract from mozilla-central the standalone content into the loop-client git repo. r=natim,dmose
@Standard8 Standard8 merged commit 9388311 into mozilla:master Aug 12, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants