GitReleaseLog #175

shresthagrawal · 2019-03-09T17:20:14Z

This PR is still incomplete!
Solves #91

To test this, copy gitchangelog/.gitchangelog.rc and gitchangelog/markdown.tpl into the the root dir of the project.

I need some help from maintainers regarding the following points to complete this pr:

How should I include the the not python files (.gitchangelog.rc and template.tpl) into the package and automatically copy it using python to the repo. (This would help in solving release-bot init #168 also)
How should the change log look like by default? This would help me create the template for the gitchangelog and also currently the changelogs are divided into parts(New, Changes, Fix, Others) Do we need that?

sidenote: I have also included the commits from my previous PR because that bug was creating a problem in GitChangeLog implementation.

centos-ci · 2019-03-09T17:20:16Z

Can one of the admins verify this patch?

requirements.txt

TomasTomecek · 2019-03-10T11:33:21Z

@user-cont/the-team hey guys, could you please review and reply to Shresth? I'd like to know your opinion before I state mine.

lachmanfrantisek · 2019-03-11T09:47:08Z

How should I include the the not python files (.gitchangelog.rc and template.tpl) into the package and automatically copy it using python to the repo. (This would help in solving #168 also)

In setup.py (or setup.cfg), you can specify data_files. (example in user-cont/colin)

lachmanfrantisek · 2019-03-11T09:55:15Z

How should the change log look like by default?

I vote for something simillar to what we have now, so we can easily continue to use this. (Ideally, we can have same format for changelog as well as for git release, so markdown and new/changes/.. parts would be nice.)

This would help me create the template for the gitchangelog and also currently the changelogs are divided into parts(New, Changes, Fix, Others) Do we need that?

It's usefull and it's a common practice. The gitchangelog supports that:

Feature

classify commit message into sections (ie: New, Fix, Changes...)

rpitonak · 2019-03-11T10:59:17Z

How should the change log look like by default? This would help me create the template for the gitchangelog and also currently the changelogs are divided into parts(New, Changes, Fix, Others) Do we need that?

+1 for having sections.

TomasTomecek · 2019-03-11T14:15:00Z

As for distributing the files, you can also put them into python code as multiline strings.

release_bot/configuration.py

release_bot/git.py

release_bot/init_repo.py

TomasTomecek

Very nice progress. I'd personally prefer if the init change was done as a new PR. But never mind now.

TomasTomecek · 2019-03-12T18:57:33Z

release_bot/cli.py

@@ -31,6 +31,8 @@ def parse_arguments():
                            default='')
        parser.add_argument("-v", "--version", help="display program version", action='version',
                            version=f"%(prog)s {configuration.version}")
+        parser.add_argument("-i", "--init", help="initializes the repo to be used by the release_bot",


please do a subcommand, not an option:

$ releasebot init ...

release_bot/init_repo.py

TomasTomecek · 2019-03-12T19:03:55Z

release_bot/init_repo.py

+        """
+        Creates template file for the Markdown output
+        """
+        template_string ="""{{#general_title}}


can we store this in the top of the file?

TomasTomecek · 2019-03-12T19:04:26Z

release_bot/init_repo.py

+
+            {{/versions}}"""
+        # This removes the tabed spaces from the template_string
+        template_string = template_string.replace('            ','')


eiwwww, just format the string correctly in the first place

shresthagrawal · 2019-03-13T11:31:44Z

Just a few more suggestions from maintainers.

Before running the init command should the bot check that the dir from which the command is called is an valid git repo.
At the end of the script should the bot also commit all the changes to the repo ?

release_bot/init_repo.py

shresthagrawal · 2019-03-15T08:01:46Z

Hey can someone from the maintainers review the PR as I have made all of the changes requested! Thanks

release_bot/init_repo.py

release_bot/cli.py

release_bot/init_repo.py

jpopelka

@TomasTomecek check your requested changes

rpitonak

Just nits, not blocking the merge.

rpitonak · 2019-03-18T08:46:02Z

README.md

@@ -140,6 +167,13 @@ Here are possible options:

 Sample config named [release-conf-example.yaml](release-conf-example.yaml) can be found in this repository.

+## GitChangeLog
+
+For using the [gitchangelog](https://github.com/vaab/gitchangelog) you must add the line `gitchanelog = true` to the conf.yaml, and add the files `.gitchangelog.rc` and `markdown.tpl` in the root of your upstream project repository. Sample config files: [.gitchangelog.rc](/gitchangelog/.gitchangelog.rc) and [template.tpl](/gitchangelog/template.tpl).


gitchanelog = true -> gitchanelog : true

rpitonak · 2019-03-18T08:48:08Z

release_bot/cli.py

+                if not args.configuration.is_file():
+                    configuration.logger.error(
+                        f"Supplied configuration file is not found: {args.configuration}")
+                    exit(1)


Would not be better to raise an exception here?

rpitonak · 2019-03-18T08:56:48Z

release_bot/init_repo.py

+        """
+        self.conf['repository_name'] = input('Please enter the repository name:')
+        self.conf['repository_owner'] = input('Please enter the repository owner:')
+        print("""for details on how to get github token checkou


checkou -> checkout

rpitonak · 2019-03-18T08:58:49Z

release_bot/init_repo.py

+        """
+        Create the release-conf.yaml and conf.yaml
+        """
+        self.conf['repository_name'] = input('Please enter the repository name:')


I would like to have some validation of input but could be done in follow-up PRs

TomasTomecek · 2019-03-18T17:40:04Z

I just tried this and here is my observation:

Why's gitchangelog in the secret config file and not in the release-conf?
The prompting part is inconsistent (some lines start with a big letter, some don't)
The prompting part is hard to read - a wall of text: I would much prefer if the init command created the files for me and then wrote instructions how to update them instead of prompting me: I hate prompts. I guess this could configurable: default to the interactive prompt (which is better for new users) and offer a non-interactive run.
Then I was finally able to run release bot and this was the result: 0.1.0 release packit/ogr#38, no idea why the changelog is empty: I didn't find any clues in the debug output - no idea what went wrong.

Edit: figured it out: mustache was not installed, I have a changelog now: packit/ogr#39
Edit2: The changelog generated by gitchangelog is better than what we had but still needs some tinkering (the template mainly), we can do that in next PRs.

And finally, I had to signifiicantly patch release-bot to make it run - the CLI code seems to be pretty borked.

diff --git a/release_bot/cli.py b/release_bot/cli.py
index e18e32c..be21d12 100644
--- a/release_bot/cli.py
+++ b/release_bot/cli.py
@@ -37,15 +37,6 @@ class CLI:
             """
             Makes the bot to perform normal release tasks
             """
-            if args.configuration:
-                args.configuration = Path(args.configuration).resolve()
-                if not args.configuration.is_file():
-                    raise ReleaseException(
-                        f"Supplied configuration file is not found: {args.configuration}")
-                    if args.debug:
-                        configuration.logger.setLevel(logging.DEBUG)
-                        for key, value in vars(args).items():
-                            setattr(configuration, key, value)
 
         parser = argparse.ArgumentParser(description="Automatic releases bot", prog='release-bot')
         parser.add_argument("-d", "--debug", help="turn on debugging output",
@@ -62,4 +53,13 @@ class CLI:
         parser_init.set_defaults(func=run_init)
 
         args = parser.parse_args()
+        if args.debug:
+            configuration.logger.setLevel(logging.DEBUG)
+            for key, value in vars(args).items():
+                setattr(configuration, key, value)
+        if args.configuration:
+            configuration.configuration = Path(args.configuration).resolve()
+            if not configuration.configuration.is_file():
+                raise ReleaseException(
+                    f"Supplied configuration file is not found: {args.configuration}")
         args.func(args)

I understand that argparse is hard to work with, feel free to get inspired here.

TomasTomecek

I understand that argparse is hard to work with, please see my patch above and a link how you can easily do subcommands with argparse.

TomasTomecek · 2019-03-18T17:12:38Z

release_bot/cli.py

+                if not args.configuration.is_file():
+                    raise ReleaseException(
+                        f"Supplied configuration file is not found: {args.configuration}")
+                    if args.debug:


This is deadcode and debug logging is never set.

Hey, I just dont know how and why I did this mistake, will correct it!

Okay while I created the function run_bot the indentation was mistakened while copying! Really sorry for it

I just found a bug in Atom, if you move lines up or down using ctrl it just automatically changes the indentation :(

TomasTomecek · 2019-03-18T17:17:06Z

release_bot/cli.py

+                        f"Supplied configuration file is not found: {args.configuration}")
+                    if args.debug:
+                        configuration.logger.setLevel(logging.DEBUG)
+                        for key, value in vars(args).items():


This should not be in the if debug branch.

I understand that argparse is hard to work with, please see my patch above and a link how you can easily do subcommands with argparse.

Hey did you remove the patch?? When I first saw your review there was some code edits ?

Thanks @TomasTomecek will work on it

By patch I meant the diff in the big comment above.

@TomasTomecek, I think this is better!

TomasTomecek · 2019-03-27T08:44:21Z

needs rebase

shresthagrawal · 2019-04-17T18:16:39Z

Hey, Sorry I was really busy on some other project! how should I go forward with this pull request? Should I rebase?

jpopelka · 2019-04-18T06:08:34Z

Should I rebase?

That would be a good (re-)start, yes, please.

shresthagrawal · 2019-04-27T10:13:41Z

Is this better? I am not a pro at git rebase so it might be that I would have messed up something! Let me know if any further changes are required!

TomasTomecek · 2019-04-29T12:49:17Z

As github says, there are still conflicts which need to be addressed.

You first need to fetch latest changes from upstream master and then do git rebase upstream/master.

The rebase procedure is explained really well here: https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request

TomasTomecek · 2019-05-15T20:36:12Z

Sorry for a lack of response, but most of the team was busy with various tasks lately.

I just tried this locally and sadly it doesn't work:

$ release-bot init
Traceback (most recent call last):
  File "/home/tt/.local/bin//release-bot", line 10, in <module>
    sys.exit(main())
  File "/home/tt/.local/lib/python3.7/site-packages/release_bot/releasebot.py", line 314, in main
    init_repo.run(args.silent)
NameError: name 'init_repo' is not defined

Seems like you made a small mistake when rebasing, can be easily solved, see mu suggest. Otherwise this works fine. LGTM

release_bot/releasebot.py

Co-Authored-By: Tomas Tomecek <tomas@tomecek.net>

rpitonak

LGTM, thank you!

TomasTomecek reviewed Mar 10, 2019

View reviewed changes

requirements.txt Show resolved Hide resolved

shresthagrawal mentioned this pull request Mar 11, 2019

release-bot init #168

Open

jpopelka reviewed Mar 12, 2019

View reviewed changes

TomasTomecek suggested changes Mar 12, 2019

View reviewed changes

jpopelka reviewed Mar 13, 2019

View reviewed changes

release_bot/init_repo.py Outdated Show resolved Hide resolved

release_bot/init_repo.py Outdated Show resolved Hide resolved

jpopelka reviewed Mar 15, 2019

View reviewed changes

release_bot/init_repo.py Outdated Show resolved Hide resolved

release_bot/cli.py Outdated Show resolved Hide resolved

release_bot/init_repo.py Show resolved Hide resolved

jpopelka approved these changes Mar 18, 2019

View reviewed changes

rpitonak reviewed Mar 18, 2019

View reviewed changes

shresthagrawal force-pushed the release_log branch from f14d5ff to 72b6b38 Compare March 18, 2019 16:54

TomasTomecek suggested changes Mar 18, 2019

View reviewed changes

TomasTomecek mentioned this pull request Mar 27, 2019

[RFE] Add support for towncrier #199

Open

shresthagrawal force-pushed the release_log branch from 6dbf869 to 2029187 Compare April 27, 2019 10:11

Add support for GitChangeLog and subcommand release-bot init.

e098d2d

shresthagrawal force-pushed the release_log branch from 2029187 to e098d2d Compare May 3, 2019 19:07

correct tabs

061bf93

TomasTomecek suggested changes May 15, 2019

View reviewed changes

release_bot/releasebot.py Outdated Show resolved Hide resolved

Update release_bot/releasebot.py

5b962f8

Co-Authored-By: Tomas Tomecek <tomas@tomecek.net>

TomasTomecek approved these changes May 15, 2019

View reviewed changes

rpitonak approved these changes May 20, 2019

View reviewed changes

rpitonak merged commit 4b537c6 into user-cont:master May 20, 2019

GitReleaseLog #175

GitReleaseLog #175

Conversation

shresthagrawal commented Mar 9, 2019 • edited Loading

centos-ci commented Mar 9, 2019

TomasTomecek commented Mar 10, 2019

lachmanfrantisek commented Mar 11, 2019

lachmanfrantisek commented Mar 11, 2019

rpitonak commented Mar 11, 2019

TomasTomecek commented Mar 11, 2019

TomasTomecek 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

shresthagrawal commented Mar 13, 2019

shresthagrawal commented Mar 15, 2019

jpopelka left a comment

Choose a reason for hiding this comment

rpitonak 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

TomasTomecek commented Mar 18, 2019 • edited Loading

TomasTomecek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shresthagrawal Mar 18, 2019 • 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

shresthagrawal Mar 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomasTomecek commented Mar 27, 2019

shresthagrawal commented Apr 17, 2019

jpopelka commented Apr 18, 2019

shresthagrawal commented Apr 27, 2019

TomasTomecek commented Apr 29, 2019

TomasTomecek commented May 15, 2019

rpitonak left a comment

Choose a reason for hiding this comment

shresthagrawal commented Mar 9, 2019 •

edited

Loading

TomasTomecek commented Mar 18, 2019 •

edited

Loading

shresthagrawal Mar 18, 2019 •

edited

Loading

shresthagrawal Mar 18, 2019 •

edited

Loading