Skip to content

Feature/adds release display-name flag #1110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
murilovarela wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Feature/adds release display-name flag #1110

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec1731a
Adds display-name flag to release command
Jul 21, 2021
58e2ad5
Uses @ to separate app name and version
Jul 27, 2021
725be63
Removes v when using --displayName
Jul 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- `--display-name` flag to release command

@estacioneto estacioneto Jul 27, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Requesting changes just to block the PR so this comment can be seen before the merge 😅 )

@gris, I've talked to @murilovarela and we thought about adding the vendor to the tag name. This might be useful to also quickly install an app just by copying the tag name. What do you think?

@gris gris Jul 27, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you guys think that it is a cool idea, I'm all in!
Just out of curiosity did you guys evaluated the usage of lerna, bazel or some other monorepo tooling?
I've just noticed that having a tag referring to a specific app inside a monorepo can be misleading, specially if you are updating more than 1 app at the same time (you will have to push 2 tags pointing to the same commit).

@murilovarela murilovarela Jul 28, 2021

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I see we will have two commits with the version bump change, one for each app, and the tag will be pointing to that specific commit. 🤔

@estacioneto estacioneto Jul 28, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you talk about the work you're doing in the CI bot, @murilovarela?

@gris gris Jul 28, 2021 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, I see. I was just wondering if it makes more sense to have the tags point to versions of the monorepo itself, like this https://github.com/vtex/faststore/blob/master/lerna.json and https://github.com/vtex/faststore/tags

@gris gris Jul 28, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I've researched a little bit more and actually saw this process implemented with tags per package in other places. I'm very curious because I will implement a monorepo for my team in the future as well :D

@murilovarela murilovarela Jul 28, 2021

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI bot doesn't use the toolbelt. It generates the tag by itself. And what we decide here, I will replicate in the ci-hub code.

@murilovarela murilovarela Jul 28, 2021

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we all agree that it is a good idea to add the vendor to the tag name, correct?

@estacioneto estacioneto Jul 28, 2021 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, @murilovarela. I meant to share knowledge about what we're doing since @gris is curious about the monorepo initiative.

Anyway, could you add the vendor in the tag name here too? 😁

Edit: Commented before seeing the above comment 😅

@gris gris Jul 28, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, it makes sense to have the vendor for me.


## [3.8.0-beta] - 2021-06-09

### Removed
Expand Down
1 change: 1 addition & 0 deletions docs/commands.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,7 @@ OPTIONS
-h, --help show CLI help
-v, --verbose Show debug level logs
--trace Ensure all requests to VTEX IO are traced
--display-name Add the project name to the tag and release commit

EXAMPLES
vtex release
Expand Down
10 changes: 8 additions & 2 deletions src/commands/release.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { flags as oclifFlags } from '@oclif/command'

import { CustomCommand } from '../api/oclif/CustomCommand'
import appsRelease, { releaseTypeAliases, supportedReleaseTypes, supportedTagNames } from '../modules/release'

import { ColorifyConstants } from '../api/constants/Colors'

export default class Release extends CustomCommand {
Expand All @@ -17,6 +18,10 @@ export default class Release extends CustomCommand {

static flags = {
...CustomCommand.globalFlags,
'display-name': oclifFlags.boolean({
description: 'Add the project name to the tag and release commit',
default: false,
}),
}

static args = [
Expand All @@ -33,8 +38,9 @@ export default class Release extends CustomCommand {
async run() {
const {
args: { releaseType, tagName },
flags: { 'display-name': displayName },
} = this.parse(Release)

await appsRelease(releaseType, tagName)
await appsRelease(releaseType, tagName, displayName)
}
}
7 changes: 5 additions & 2 deletions src/modules/release/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import semver from 'semver'

import log from '../../api/logger'
import { ReleaseUtils } from './utils'
import { ManifestEditor } from '../../api/manifest'

export const releaseTypeAliases = {
pre: 'prerelease',
Expand Down Expand Up @@ -59,13 +60,15 @@ Valid release tags are: ${supportedTagNames.join(', ')}`)

export default async (
releaseType = 'patch', // This arg. can also be a valid (semver) version.
tagName = 'beta'
tagName = 'beta',
displayName = false
) => {
const utils = new ReleaseUtils()
utils.checkGit()
utils.checkIfInGitRepo()
const normalizedReleaseType = prop<string>(releaseType, releaseTypeAliases) || releaseType
const [oldVersion, newVersion] = getNewAndOldVersions(utils, normalizedReleaseType, tagName)
const manifest = await ManifestEditor.getManifestEditor()

log.info(`Old version: ${chalk.bold(oldVersion)}`)
log.info(`New version: ${chalk.bold.yellow(newVersion)}`)
Expand All @@ -75,7 +78,7 @@ export default async (
.split('/')

// Pachamama v2 requires that version tags start with a 'v' character.
const tagText = `v${newVersion}`
const tagText = `${displayName ? manifest.name : ''}/v${newVersion}`
Comment thread
estacioneto marked this conversation as resolved.
Outdated
const changelogVersion = `\n\n## [${newVersion}] - ${year}-${month}-${day}`

if (!(await utils.confirmRelease())) {
Expand Down