The regex shows the possible version part in url could be a number which is greater than 9. Whereas, api_url[0:-2] assumes the version number is only less than 10. While reading through these code, it looks kind of confusion.

My understand of these code is to remove the version number if it exists, and construct a new url with the self.module_api_version. Would it be good to just extract the url part with regex? I think that could be helpful to avoid the confusion.

Is it really necessary to clear screen (remove all previous lines in the terminal) in order to watch build?

When I ran the module build command, I feel it's wired and all previous lines in the terminal are lost which is inconvenient indeed.

This comment is about UX.

When there are multiple build IDs, then user will see the "WARNING: ..." message in screen, and the screen is frozen for a while, which is because time.sleep(3), but there is nothing to tell user what is happening and it cannot be skipped or canceled by any key press by the user. This might be a problem and make someone unhappy.

My suggestion is to print the warning message inside self.module_watch_build along with other output during the watch. What do you think?

Suggestion:

builds = data if isinstance(data, list) else [data]
return [build['id'] for build in builds]

I thought this alternative way because no need of api_version = int(api_url[-2:-1]), which [-2:-1] has same issue mentioned in another comment about git_url.

If version check is needed, I suggest to consider that comment together, to use a separate method to split a api_url into two parts (the rest characters before version, version) and reuse this method in these two places.

Nice suggestion. I'll implement that.

@cqi with the way we are currently displaying it, it would look confusing if we didn't clear the screen. There maybe alternatives, where clearing the screen is unnecessary but that is out of the scope of this PR. I'll instead move the warning to the watch function so that way the warning is displayed for 15 seconds before being cleared.

@cqi thank you for the feedback. I changed the PR so that the warning is always shown when watching a submission that resulted in several module builds.

@cqi can you please review this again?

:thumbsup:

pretty please pagure-ci rebuild

:thumbsup: Looks good to me. Thanks.

@cqi can you merge this please?

Commit 6d91d628 fixes this pull-request