Cobrify `profileBuilder` #1054

marstr · 2018-02-07T00:52:31Z

Refactoring profileBuilder to use cobra/viper. The ultimate idea being to make it easier to run.

With the addition of the defintion.go files in the profiles, we should even be able to just run:

go generate ./profiles/...

Fixes #1044

Thanks you for your contribution to the Azure-SDK-for-Go! We will triage and review it as quickly as we can.

As part of your submission, please make sure that you can make the following assertions:

I'm not making changes to Auto-Generated files which will just get erased next time there's a release.
- If that's what you want to do, consider making a contribution here: https://github.com/Azure/autorest.go
I've tested my changes, adding unit tests where applicable.
I've added Apache 2.0 Headers to the top of any new source files.
I'm submitting this PR to the dev branch, or I'm fixing a bug that warrants its own release and I'm targeting the master branch.
If I'm targeting the master branch, I've also added a note to CHANGELOG.md.
I've mentioned any relevant open issues in this PR, making clear the context for the contribution.

marstr · 2018-02-07T00:53:08Z

Known issue: for some reason it isn't respecting arguments. (Obviously I intend on fixing this before merging)

jhendrixMSFT · 2018-02-07T17:47:58Z

Let's rename the definition.go files to generate.go (there is prior art to this pattern).

jhendrixMSFT · 2018-02-07T17:50:03Z

Also can you update the README with instructions and example command lines?

vladbarosan · 2018-02-07T19:45:47Z

Like I mentioned in a previous PR, to respect the new repo status we need to use the list strategy using the file at https://github.com/Azure/azure-sdk-for-go/blob/dev/profiles/latest/stableApis.txt that gets generated when generating the code.

Refers to: profiles/preview/definition.go:4 in c426bb6. [](commit_id = c426bb6c93ac89b2bba224606b0497a1d3cfda56, deletion_comment = False)

vladbarosan · 2018-02-07T19:51:24Z

tools/profileBuilder/test.sh

@@ -1,5 +1,5 @@
 #!bin/bash
 set -ev
 if [[ ! "${TRAVIS_GO_VERSION}" < "1.9" ]]; then
-    go test -v github.com/Azure/azure-sdk-for-go/tools/profileBuilder
+    go test -v github.com/Azure/azure-sdk-for-go/tools/profileBuilder/...


can we add a go vet here also ?

Fixed in 99a687b

vladbarosan · 2018-02-07T19:52:41Z

tools/profileBuilder/model/transforms.go

+// profileBuilder.
+//
+// This package is not governed by the SemVer associated with the rest of the
+// Azure-SDK-for-Go.


missing license info

Fixed in 3debc48.

vladbarosan · 2018-02-07T19:59:30Z

tools/profileBuilder/model/transforms.go

+
+var packageName = regexp.MustCompile(`services[/\\](?P<provider>[\w\-\.\d_\\/]+)[/\\](?:(?P<arm>` + armPathModifier + `)[/\\])?(?P<version>v?\d{4}-\d{2}-\d{2}[\w\d\.\-]*|v?\d+\.\d+[\.\d\w\-]*)[/\\](?P<group>[/\\\w\d\-\._]+)`)
+
+// BuildProfile takes a list of packages and creates a profile


I presume transforms and transforms_test are mostly a copy paste from program and program_test would have been good if we can keep the history ( i.e. move and rename the file -> commit -> modify -> commit) .

Fixed in 3943639.

Fixes Azure#1044

This should considerably ease the usage of the profileBuilder.

marstr · 2018-02-08T04:37:01Z

@marstr says:
Known issue: for some reason it isn't respecting arguments. (Obviously I intend on fixing this before merging)

I worked around this in e225c66, but don't like what I did. I threw an issue in our backlog to go clean this up. (#1060)

The `profileBuilder` lives in the same repository as the Azure-SDK-for-Go, but is a logically separate enough tool that it warrants having its own `dep` manifest. For further reading on how `dep` handles multiple manifests, read here: https://golang.github.io/dep/docs/glossary.html#project-root

marstr · 2018-02-08T19:20:45Z

@jhendrixMSFT says:
Also can you update the README with instructions and example command lines?

Fixed in 531faa8

An extra special character was breaking CI. I removed it to squelch the message. Also, renaming the profile `definition.go` files to `generate.go` files as per review feedback.

marstr · 2018-02-08T19:26:21Z

@jhendrixMSFT says:
Let's rename the definition.go files to generate.go (there is prior art to this pattern).

Fixed in 9c3ee4f

jhendrixMSFT · 2018-02-08T19:30:36Z

tools/profileBuilder/model/transforms.go

+// Package model holds the business logic for the operations made available by
+// profileBuilder.
+//
+// This package is not governed by the SemVer associated with the rest of the


Should this go in the README instead? I think it gets lost among the license header. It's a small issue though.

I put it here so that it shows up in GoDoc.org, I'll copy it into the README as well though.

Fixed in d7cb677

vladbarosan

LGTM with the 2 issues remaining that were created

#1066

#1060

marstr self-assigned this Feb 7, 2018

marstr requested review from jhendrixMSFT, vladbarosan and mcardosos February 7, 2018 00:52

ghost added the review label Feb 7, 2018

vladbarosan reviewed Feb 7, 2018

View reviewed changes

marstr added 5 commits February 7, 2018 18:53

Renaming program.go to transforms.go

3943639

Updating profileBuilder to be a Cobra/Viper app.

6a8d995

Fixes Azure#1044

+Definition files to allow use of go generate

b530e73

This should considerably ease the usage of the profileBuilder.

Removing duplicate License file

ffe7c39

Adding headers

3debc48

marstr force-pushed the cobrify branch from c426bb6 to 3debc48 Compare February 8, 2018 02:58

Working around Azure#1060

e225c66

Fixing License Headers to match CI Regex

9c3ee4f

An extra special character was breaking CI. I removed it to squelch the message. Also, renaming the profile `definition.go` files to `generate.go` files as per review feedback.

Adding go vet to profileBuilder tests.

99a687b

jhendrixMSFT reviewed Feb 8, 2018

View reviewed changes

Adding version disclaimer to profileBuilder README

d7cb677

vladbarosan mentioned this pull request Feb 8, 2018

Properly generate latest profile #1066

Closed

vladbarosan approved these changes Feb 8, 2018

View reviewed changes

jhendrixMSFT approved these changes Feb 8, 2018

View reviewed changes

marstr merged commit 396b427 into Azure:dev Feb 8, 2018

ghost removed the review label Feb 8, 2018

marstr deleted the cobrify branch February 15, 2018 22:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cobrify `profileBuilder` #1054

Cobrify `profileBuilder` #1054

marstr commented Feb 7, 2018 •

edited

Loading

marstr commented Feb 7, 2018

jhendrixMSFT commented Feb 7, 2018

jhendrixMSFT commented Feb 7, 2018

vladbarosan commented Feb 7, 2018

vladbarosan Feb 7, 2018

marstr Feb 8, 2018

vladbarosan Feb 7, 2018

marstr Feb 8, 2018

vladbarosan Feb 7, 2018

marstr Feb 8, 2018 •

edited

Loading

marstr commented Feb 8, 2018

marstr commented Feb 8, 2018

marstr commented Feb 8, 2018

jhendrixMSFT Feb 8, 2018

marstr Feb 8, 2018

marstr Feb 8, 2018

vladbarosan left a comment


		var packageName = regexp.MustCompile(`services[/\\](?P<provider>[\w\-\.\d_\\/]+)[/\\](?:(?P<arm>` + armPathModifier + `)[/\\])?(?P<version>v?\d{4}-\d{2}-\d{2}[\w\d\.\-]\|v?\d+\.\d+[\.\d\w\-])[/\\](?P<group>[/\\\w\d\-\._]+)`)

		// BuildProfile takes a list of packages and creates a profile

Cobrify profileBuilder #1054

Cobrify profileBuilder #1054

Conversation

marstr commented Feb 7, 2018 • edited Loading

marstr commented Feb 7, 2018

jhendrixMSFT commented Feb 7, 2018

jhendrixMSFT commented Feb 7, 2018

vladbarosan commented Feb 7, 2018

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

marstr Feb 8, 2018 • edited Loading

Choose a reason for hiding this comment

marstr commented Feb 8, 2018

marstr commented Feb 8, 2018

marstr commented Feb 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladbarosan left a comment

Choose a reason for hiding this comment

Cobrify `profileBuilder` #1054

Cobrify `profileBuilder` #1054

marstr commented Feb 7, 2018 •

edited

Loading

marstr Feb 8, 2018 •

edited

Loading