Sync with playbooks: install-go-tools, gotestsum, and dynamic versions #192

lieut-data · 2023-12-12T22:30:51Z

Summary

This brings into the starter-template a handful of changes originally introduced with Playbooks:

an automatic install-go-tools target that manages a local, versioned copy of any go installed tools needed
adoption of gotestsum for improved go testing semantics
reverting the embedding of plugin.json, restoring make apply, and shifting to a dynamically generated version number based on the nearest tag.

This last point is easier explained by linking to the original PR: mattermost/mattermost-plugin-playbooks#433, but I note that the reason I reverted embedding plugin.json is that this root file is no longer the final source of truth: rather, it's a (big) input into the final manifest that includes the dynamically generated version. Open to different approaches, but either way we don't have to care about checking in the generated manifests: they remain generated and .gitignored.

In this repository, since we have yet to tag a version, any build will result in v0.0.0+<commit hash>. Any build on a tagged version will be vX.Y.Z exactly. And any downstream commit from there would be vX.Y.Z+<short hash> until we tag a new version.

Ticket Link

None.

This reverts commit be4a281.

This reverts commit ca9ee3c.

This reverts commit 18d30b5.

codecov-commenter · 2023-12-12T22:34:29Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (376ea7e) 5.26% compared to head (0ac4026) 5.26%.

Additional details and impacted files

@@          Coverage Diff           @@
##           master    #192   +/-   ##
======================================
  Coverage    5.26%   5.26%           
======================================
  Files           3       3           
  Lines          38      38           
======================================
  Hits            2       2           
  Misses         36      36

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cpoile

I think this is a good way to do it, nice!

build/manifest/main.go

mickmister · 2023-12-13T21:02:36Z

build/manifest/main.go

+	// Update the manifest based on the state of the current commit
+	// Use the first version we find (to prevent causing errors)
+	var version string
+	tags := strings.Fields(BuildTagCurrent)
+	for _, t := range tags {
+		if strings.HasPrefix(t, "v") {
+			version = t
+			break
+		}
+	}
+	if version == "" {
+		if BuildTagLatest != "" {
+			version = BuildTagLatest + "+" + BuildHashShort
+		} else {
+			version = "v0.0.0+" + BuildHashShort
+		}
+	}
+	manifest.Version = strings.TrimPrefix(version, "v")
+
+	// Update the release notes url to point at the latest tag, if present.
+	if BuildTagLatest != "" {
+		manifest.ReleaseNotesURL = manifest.HomepageURL + "releases/tag/" + BuildTagLatest
+	}
+


LGTM 👍

If I understand correctly:

if the current commit matches a tag, then we use that

If not, use the latest tag and append the latest commit hash

If no tag available, use v0.0.0 and append the latest commit hash

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>

hanzei

Thanks for improving the starter template 🚀

hanzei · 2023-12-18T15:27:06Z

Makefile

+## Install go tools
+install-go-tools:
+	@echo Installing go tools
+	$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1
+	$(GO) install gotest.tools/gotestsum@v1.7.0


I really like that plugin now own the version of golangci-lint.

hanzei · 2023-12-18T15:28:01Z

Makefile

+## Install go tools
+install-go-tools:
+	@echo Installing go tools
+	$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1


What is the impact of this change on CI? AFAIK it also installs a version of golangci-lint. Can we drop that?

See below: uncertain how to roll out these changes to CI without breaking everything. For now, this at least gives the developer a reliable definition of the version in play. Thoughts?

hanzei · 2023-12-18T15:31:01Z

Makefile

+## Runs any lints and unit tests defined for the server and webapp, if they exist, optimized
+## for a CI environment.
+.PHONY: test-ci
+test-ci: apply webapp/node_modules install-go-tools


Is there a follow up ticket to actually use the target in CI?

Not at the moment, but I'm a bit blurry on how to achieve that -- it looks like we import @main, which would make whatever changes we add there affect all plugins at the same time, vs. plugins opting in to updated CI incrementally. Not sure how to crack this nut: thoughts?

Maybe it's time to move to hand-picked CI versions. We have been bitten by the auto-update of @main way too often. Espcecially, updates to golangci-lint caused issues in the past.

cc @mickmister

@mickmister Would you be open to owning that change? Brightscout could surely help with executing it.

build/setup.mk

build/manifest/main.go

hanzei

A few open discussion points, but nothing blocking

build/manifest/main.go

hanzei · 2024-01-23T12:22:32Z

Makefile

+## Install go tools
+install-go-tools:
+	@echo Installing go tools
+	$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1


Do we actually need to install the binary? Or is

Suggested change

$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1

$(GO) run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1

enough?

Oh, that would be convenient! But unfortunately much slower it seems?

Well, the added duration comes from checking the remote go modules DB. The increase is similar running go install.

One downside of go run is, that it errors if the machine is offline.

Ah, you're right: I wasn't comparing apples to apples as I thought!

hanzei · 2024-01-23T12:24:47Z

Makefile

+## Runs any lints and unit tests defined for the server and webapp, if they exist, optimized
+## for a CI environment.
+.PHONY: test-ci
+test-ci: apply webapp/node_modules install-go-tools


Maybe it's time to move to hand-picked CI versions. We have been bitten by the auto-update of @main way too often. Espcecially, updates to golangci-lint caused issues in the past.

cc @mickmister

lieut-data added 5 commits December 12, 2023 16:33

Revert "Update main.go (#154)"

6158e72

This reverts commit be4a281.

Revert "[MM-33506] Use embed package to include plugin manifest (#145)"

5463bc1

This reverts commit ca9ee3c.

Revert "Don't generate manifest.ts (#127)"

f927586

This reverts commit 18d30b5.

install-go-tools target, adopt gotestsum

42f5eaf

bring back make apply + automatic versioning

0ac4026

lieut-data added the 2: Dev Review Requires review by a core committer label Dec 12, 2023

lieut-data requested review from cpoile, mickmister and hanzei December 12, 2023 22:30

lieut-data mentioned this pull request Dec 12, 2023

Sync with starter template mattermost/mattermost-plugin-playbooks#1887

Merged

lieut-data mentioned this pull request Dec 12, 2023

Sync with starter template: dynamic versions mattermost/mattermost-plugin-msteams#447

Merged

cpoile approved these changes Dec 13, 2023

View reviewed changes

mickmister approved these changes Dec 13, 2023

View reviewed changes

Update build/manifest/main.go

0902371

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>

hanzei requested changes Jan 8, 2024

View reviewed changes

lieut-data added 3 commits January 12, 2024 10:45

suppress git describe error when no tags match

14a8271

make version/release notes opt-in

ef17405

fix whitespace in Makefile

1767c29

lieut-data requested a review from hanzei January 12, 2024 15:06

hanzei approved these changes Jan 23, 2024

View reviewed changes

document version management options

175bc99

lieut-data added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Jan 23, 2024

lieut-data enabled auto-merge (squash) January 23, 2024 14:00

lieut-data merged commit de0b31b into master Jan 23, 2024
5 checks passed

lieut-data deleted the sync-with-playbooks branch January 23, 2024 14:02

ayusht2810 mentioned this pull request Jan 31, 2024

[MM-176] Sync the plugin with starter template mattermost/mattermost-plugin-gitlab#451

Merged

hanzei mentioned this pull request Mar 25, 2024

Downstream latest changes from mattermost-plugin-starter-template mattermost/mattermost-plugin-google-calendar#71

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync with playbooks: install-go-tools, gotestsum, and dynamic versions #192

Sync with playbooks: install-go-tools, gotestsum, and dynamic versions #192

lieut-data commented Dec 12, 2023 •

edited

codecov-commenter commented Dec 12, 2023

cpoile left a comment

mickmister Dec 13, 2023

lieut-data Dec 13, 2023

hanzei left a comment

hanzei Dec 18, 2023 •

edited

hanzei Dec 18, 2023

lieut-data Jan 12, 2024

hanzei Dec 18, 2023

lieut-data Jan 12, 2024

hanzei Jan 23, 2024

hanzei Jan 26, 2024

hanzei left a comment

hanzei Jan 23, 2024

lieut-data Jan 23, 2024

hanzei Jan 26, 2024

lieut-data Jan 26, 2024

hanzei Jan 23, 2024

	$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1
	$(GO) run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1

Sync with playbooks: install-go-tools, gotestsum, and dynamic versions #192

Sync with playbooks: install-go-tools, gotestsum, and dynamic versions #192

Conversation

lieut-data commented Dec 12, 2023 • edited

Summary

Ticket Link

codecov-commenter commented Dec 12, 2023

Codecov Report

cpoile left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanzei left a comment

Choose a reason for hiding this comment

hanzei Dec 18, 2023 • edited

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

hanzei 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

lieut-data commented Dec 12, 2023 •

edited

hanzei Dec 18, 2023 •

edited