Update release script (issue 177) #487

alixdamman · 2017-10-17T15:36:13Z

I would like to proceed step by step.
I started with update conda-forge feedstock to trigger new packages (see #177).

alixdamman · 2017-10-18T08:09:35Z

Is there an equivalent for wget in windows?
I can download the tar file of the last release with urllib.request.urlopen and then calculate the sha256 but it is different from the one calculated with the tar file downloaded by hand from github ??? Although the size and content seems to be the same.

gdementen · 2017-10-17T18:33:19Z

make_release.py

+        with request.urlopen(url) as response, open(tar_file, 'wb') as out_file:
+            print('File {} downloaded'.format(url))
+            copyfileobj(response, out_file)
+            sha256 = call('openssl sha256 {}'.format(tar_file)).split('=')[1].strip()


use hashlib (https://docs.python.org/2/library/hashlib.html) ?

gdementen · 2017-10-17T18:36:08Z

make_release.py

@@ -340,23 +397,25 @@ def make_release(release_name=None, branch='master'):
        chdir('build')
        # ---------- #

-        do('Tagging release', call, 'git tag -a v%(name)s -m "tag release %(name)s"' % {'name': release_name})
+        do('Tagging release', call, 'git tag -a v{name} -m "tag release {name}"'.format(name=release_name))


I do not use "v" in tags for larray

gdementen · 2017-10-18T08:44:50Z

also I realize now that the make_release.py script is based on a very old version of the script from LIAM2 (liam2/liam2@a3bc927#diff-e94194a6680f2a567bea5103af5fe9cf). The relevant parts should be updated using the relevant changes until the latest version:

liam2/liam2@a3bc927...master#diff-e94194a6680f2a567bea5103af5fe9cf

Of note is update_changelog, but also some generic functions and a few bits here and there which were made more robust.

alixdamman · 2017-10-18T16:00:47Z

updated

gdementen

nice work overall!

gdementen · 2017-10-19T07:23:45Z

make_release.py

+    chdir(context['repository'])
+    version = short(context['release_name'])
+    do('Updating larrayenv metapackage to version {version}'.format(version=version), call,
+       'make_meta_package.bat {version}'.format(version=version))


I wonder if there is any use to keep the .bat file. We could move the command in here.

gdementen · 2017-10-19T07:24:50Z

make_release.py

@@ -539,6 +548,9 @@ def cleanup(context):
    (pull, ''),
    # >>> need internet from here
    (push, ''),
+    # assume the tar archive for the new release exists
+    (update_metapackage, ''),


making the conda packages themselves before the metapackage is required.

gdementen · 2017-10-19T09:27:01Z

make_release.py

+    meta_file = r'condarecipe\larray\meta.yaml'
+    with open(meta_file) as f:
+        lines = f.readlines()
+        lines[2] = "  version: {}\n".format(version)


this seems fragile, see below.

gdementen · 2017-10-19T09:27:12Z

make_release.py

+    with open(meta_file) as f:
+        lines = f.readlines()
+        lines[2] = "  version: {}\n".format(version)
+        lines[5] = "  git_tag: {}\n".format(version)


too fragile to my taste. See below.

gdementen · 2017-10-19T09:30:34Z

make_release.py

+    with open(init_file) as f:
+        lines = f.readlines()
+        for i, line in enumerate(lines[:]):
+            if '__version__' in line:


I would make a generic function for replacing lines in a file and use it for meta, init.py and setup.py

def replace_lines(fpath, [(substring_to_find, new_line), (sub2, newline2)]): pass

gdementen · 2017-10-19T09:42:16Z

make_release.py

+               'tmp_dir': TMP_PATH,
+               'build_dir': os.path.join(TMP_PATH, 'build'),
+               'public_release': public_release}
+    context_conda = {'branch': branch,


I would rather have a single context and a single tmp_dir. Add the necessary keys for the feedstock part. e.g.

'feedstock_repository': CONDA_LARRAY_FEEDSTOCK_REP, 'feedstock_build': os.path.join(TMP_PATH, 'feedstock')

you will need to either inline the clone_repository function in update_conda_forge_package or split clone_repository into:

def git_clone(repository, branch, build_dir): ...

and

def clone_source_repository(context): return git_clone(**context)

and have update_conda_forge_package call git_clone directly.

alixdamman · 2017-10-19T13:46:56Z

make_release.py


-Released on {date}.
+def create_source_archive(release_name, rev):
+    call(r'git archive --format zip --output ..\larray-{}-src.zip {}'.format(release_name, rev))


using sdist instead.

gdementen · 2017-10-23T13:36:31Z

make_release.py

-
-def update_changelog(release_name):
-    fname = relname2fname(release_name)
+def replace_lines(fpath, changes, end="\n"):


Why have an end argument? Is there a case where it is needed?

Not now but it might.

gdementen · 2017-10-23T13:39:34Z

make_release.py

+        lines = f.readlines()
+        for i, line in enumerate(lines[:]):
+            for substring_to_find, new_line in changes:
+                if substring_to_find in line and '#' not in line:


I guess the '#' not in line is meant to avoid commented lines. In that case I would use something like line.strip().startswith('#') to avoid skipping a "used" line with a comment at the end of the line.

gdementen · 2017-10-23T13:43:20Z

make_release.py

+    ############################################
+    # UPDATE LARRAY METAPACKAGE ON CONDA-FORGE #
+    ############################################
+    # assume larray package on cond-forge has been updated


conda-forge

gdementen · 2017-10-23T13:46:32Z

next_release.py

+def add_release(release_name, branch='master'):
+    update_changelog(release_name)
+    context = {'branch': branch,
+               'release_name': 'dev',


I would rather have something more descriptive here. Something like release_name+'dev'

gdementen · 2017-10-25T08:08:18Z

make_release.py

@@ -606,6 +621,7 @@ def announce_new_release(context):
    (pull, ''),
    # >>> need internet from here
    (push, ''),
+    (push_on_pypi, 'Pushing on Pypi'),


I would be nice to have a "push on pypi test server" step before, and potentially split the "push_on_pypi" step into "create_pypi_packages" and "push_on_pypi", so that we can check packages before they are uploaded. This could come later though.

gdementen · 2017-10-25T08:42:19Z

FWIW, here is what I used for the liam2 announce:
https://github.com/liam2/website/blob/master/make_release.py#L220
https://github.com/liam2/website/blob/master/make_release.py#L446

gdementen · 2017-10-25T12:22:31Z

The comments from my review from 2 days ago still stand.

gdementen · 2017-10-26T09:49:42Z

next_release.py

+               'repository': abspath(dirname(__file__)),
+               'build_dir': abspath(dirname(__file__)),
+               'public_release': True}
+    update_version(context)


would it be hard to create a single commit for update_changelog & update_version? If you can do that in less than 10min, then that's probably worth it (cleaner history), otherwise nevermind.

alixdamman · 2017-10-26T11:30:10Z

Squash and merge I guess? I prefer to not use fix 177 in the commit message since it has not been tested in real life yet and current version of make_release.py does not include larray-editor, larray-eurostat and larrayenv.

gdementen · 2017-10-26T13:21:51Z

yes, squash & merge indeed :)

… 177 for more details)

gdementen · 2017-10-26T14:06:14Z

nice work!

alixdamman requested a review from gdementen October 17, 2017 15:36

alixdamman force-pushed the update_release_script_177 branch 2 times, most recently from 0f72093 to 29d4826 Compare October 18, 2017 07:50

gdementen reviewed Oct 18, 2017

View reviewed changes

alixdamman force-pushed the update_release_script_177 branch 3 times, most recently from c7edf0e to 22710d1 Compare October 18, 2017 15:58

gdementen reviewed Oct 19, 2017

View reviewed changes

alixdamman force-pushed the update_release_script_177 branch 4 times, most recently from 86144d5 to 5dd6c31 Compare October 19, 2017 13:25

alixdamman commented Oct 19, 2017

View reviewed changes

alixdamman force-pushed the update_release_script_177 branch from 5dd6c31 to 5712808 Compare October 20, 2017 14:53

gdementen reviewed Oct 23, 2017

View reviewed changes

alixdamman force-pushed the update_release_script_177 branch from 16821f8 to e38ed5f Compare October 25, 2017 07:58

gdementen reviewed Oct 25, 2017

View reviewed changes

gdementen added this to the 0.27 milestone Oct 25, 2017

alixdamman removed this from the 0.27 milestone Oct 26, 2017

gdementen approved these changes Oct 26, 2017

View reviewed changes

alixdamman force-pushed the update_release_script_177 branch from 1499812 to 68cb9ea Compare October 26, 2017 11:27

alixdamman force-pushed the update_release_script_177 branch from 68cb9ea to d81f46f Compare October 26, 2017 11:55

updated release script: make_relase.py and next_release.py (see issue…

52e0094

… 177 for more details)

alixdamman force-pushed the update_release_script_177 branch from d81f46f to 52e0094 Compare October 26, 2017 13:58

alixdamman merged commit a358cec into larray-project:master Oct 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update release script (issue 177) #487

Update release script (issue 177) #487

alixdamman commented Oct 17, 2017

alixdamman commented Oct 18, 2017

gdementen Oct 17, 2017

gdementen Oct 17, 2017

gdementen commented Oct 18, 2017

alixdamman commented Oct 18, 2017

gdementen left a comment

gdementen Oct 19, 2017

gdementen Oct 19, 2017

gdementen Oct 19, 2017

gdementen Oct 19, 2017

gdementen Oct 19, 2017

gdementen Oct 19, 2017

alixdamman Oct 19, 2017

gdementen Oct 19, 2017 •

edited

Loading

gdementen Oct 23, 2017

alixdamman Oct 26, 2017

gdementen Oct 23, 2017

gdementen Oct 23, 2017

gdementen Oct 23, 2017

gdementen Oct 25, 2017

gdementen commented Oct 25, 2017

gdementen commented Oct 25, 2017

gdementen Oct 26, 2017

alixdamman commented Oct 26, 2017 •

edited

Loading

gdementen commented Oct 26, 2017

gdementen commented Oct 26, 2017

Update release script (issue 177) #487

Update release script (issue 177) #487

Conversation

alixdamman commented Oct 17, 2017

alixdamman commented Oct 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gdementen commented Oct 18, 2017

alixdamman commented Oct 18, 2017

gdementen 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

gdementen Oct 19, 2017 • edited Loading

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

gdementen commented Oct 25, 2017

gdementen commented Oct 25, 2017

Choose a reason for hiding this comment

alixdamman commented Oct 26, 2017 • edited Loading

gdementen commented Oct 26, 2017

gdementen commented Oct 26, 2017

gdementen Oct 19, 2017 •

edited

Loading

alixdamman commented Oct 26, 2017 •

edited

Loading