-
Notifications
You must be signed in to change notification settings - Fork 140
feat: add --changelog flag to requires update command #3626
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.
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
b138876
08495d9
19846e7
630c1ee
bb45b33
f27a4ae
1636c0c
90e7538
2de2e0f
aefb618
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -51,6 +51,8 @@ func setupRequiresCommand() *cobraext.Command { | |||
| updateCmd.Flags().Bool(cobraext.RequiresDryRunFlagName, false, cobraext.RequiresDryRunFlagDescription) | ||||
| updateCmd.Flags().String(cobraext.RequiresFormatFlagName, requiresFormatTable, fmt.Sprintf(cobraext.RequiresFormatFlagDescription, strings.Join(requiresFormatChoices, "|"))) | ||||
| updateCmd.Flags().Bool(cobraext.RequiresPrereleaseFlagName, false, cobraext.RequiresPrereleaseFlagDescription) | ||||
| updateCmd.Flags().Bool(cobraext.RequiresChangelogFlagName, false, cobraext.RequiresChangelogFlagDescription) | ||||
| updateCmd.Flags().String(cobraext.RequiresChangelogTypeFlagName, "", cobraext.RequiresChangelogTypeFlagDescription) | ||||
|
|
||||
| cmd := &cobra.Command{ | ||||
| Use: "requires", | ||||
|
|
@@ -70,6 +72,13 @@ const ( | |||
|
|
||||
| var requiresFormatChoices = []string{requiresFormatTable, requiresFormatJSON} | ||||
|
|
||||
| // changelogTypeChoices mirrors the changelog entry type enum defined in the package-spec | ||||
| // (spec/integration/changelog.spec.yml). The canonical validation happens when the | ||||
| // package is built and checked against the spec; this list enables early flag | ||||
| // validation. "deprecation" is intentionally excluded as it is not a valid type | ||||
| // for automated dependency-bump changelog entries. | ||||
| var changelogTypeChoices = []string{"bugfix", "enhancement", "breaking-change"} | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Taking a look to this list, there is another place where these values are king of defined. elastic-package/internal/cobraext/flags.go Line 107 in 6158fcd
I know that this is unrelated to tihs PR, but could you add I wonder if we could set the list in the description programmatically. |
||||
|
|
||||
| func requiresUpdateCommandAction(cmd *cobra.Command, _ []string) error { | ||||
| dryRun, err := cmd.Flags().GetBool(cobraext.RequiresDryRunFlagName) | ||||
| if err != nil { | ||||
|
|
@@ -86,6 +95,20 @@ func requiresUpdateCommandAction(cmd *cobra.Command, _ []string) error { | |||
| if err != nil { | ||||
| return cobraext.FlagParsingError(err, cobraext.RequiresPrereleaseFlagName) | ||||
| } | ||||
| changelogEnabled, err := cmd.Flags().GetBool(cobraext.RequiresChangelogFlagName) | ||||
| if err != nil { | ||||
| return cobraext.FlagParsingError(err, cobraext.RequiresChangelogFlagName) | ||||
| } | ||||
| changelogType, err := cmd.Flags().GetString(cobraext.RequiresChangelogTypeFlagName) | ||||
| if err != nil { | ||||
| return cobraext.FlagParsingError(err, cobraext.RequiresChangelogTypeFlagName) | ||||
| } | ||||
| if changelogType != "" && !changelogEnabled { | ||||
| return fmt.Errorf("--%s requires --%s", cobraext.RequiresChangelogTypeFlagName, cobraext.RequiresChangelogFlagName) | ||||
| } | ||||
| if changelogType != "" && !slices.Contains(changelogTypeChoices, changelogType) { | ||||
| return fmt.Errorf("unsupported changelog type %q, supported types: %s", changelogType, strings.Join(changelogTypeChoices, ", ")) | ||||
| } | ||||
|
|
||||
| packageRoot, err := packages.MustFindPackageRoot() | ||||
| if err != nil { | ||||
|
|
@@ -123,19 +146,11 @@ func requiresUpdateCommandAction(cmd *cobra.Command, _ []string) error { | |||
| return p.Proposed != "" | ||||
| }) | ||||
| if !dryRun && hasBumps { | ||||
| manifestPath := filepath.Join(packageRoot, packages.PackageManifestFile) | ||||
| manifestBytes, err := os.ReadFile(manifestPath) | ||||
| if err != nil { | ||||
| return fmt.Errorf("reading manifest file failed: %w", err) | ||||
| } | ||||
| manifestBytes, err = requiresupdates.Apply(manifestBytes, result.Proposals) | ||||
| newVersion, err := applyRequiresUpdate(packageRoot, result.Proposals, changelogEnabled, changelogType) | ||||
| if err != nil { | ||||
| return err | ||||
| } | ||||
| logger.Debugf("writing updated manifest: %s", manifestPath) | ||||
| if err := os.WriteFile(manifestPath, manifestBytes, 0o644); err != nil { | ||||
| return fmt.Errorf("writing manifest file failed: %w", err) | ||||
| } | ||||
| result.NewVersion = newVersion | ||||
| applied = true | ||||
| } | ||||
|
|
||||
|
|
@@ -155,6 +170,24 @@ func requiresUpdateCommandAction(cmd *cobra.Command, _ []string) error { | |||
|
|
||||
| if dryRun && hasBumps { | ||||
| cmd.Println("Dry run: manifest.yml was not modified") | ||||
| if changelogEnabled { | ||||
| tier := requiresupdates.AggregateTier(result.Proposals) | ||||
| next, err := requiresupdates.NextVersion(tier, packageRoot) | ||||
| if err != nil { | ||||
| return err | ||||
| } | ||||
| cmd.Printf("Dry run: would bump package version to %s and add changelog entries:\n", next) | ||||
| for _, p := range result.Proposals { | ||||
| if p.Proposed == "" { | ||||
| continue | ||||
| } | ||||
| t := changelogType | ||||
| if t == "" { | ||||
| t = requiresupdates.DefaultChangelogType(p.Tier()) | ||||
| } | ||||
| cmd.Printf(" - [%s] %s: %s -> %s\n", t, p.Package, p.Current, p.Proposed) | ||||
| } | ||||
| } | ||||
| } else if applied { | ||||
| cmd.Println("Updated manifest.yml") | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If |
||||
| } else if len(result.Proposals) == 0 && result.SkipReason == "" { | ||||
|
|
@@ -164,6 +197,37 @@ func requiresUpdateCommandAction(cmd *cobra.Command, _ []string) error { | |||
| return nil | ||||
| } | ||||
|
|
||||
| // applyRequiresUpdate orchestrates the two-step write: first updates requires | ||||
| // pins via requiresupdates.Apply, then (when changelogEnabled) bumps the package | ||||
| // version and patches changelog.yml via requiresupdates.ApplyChangelog. | ||||
| // Returns the new package version when --changelog bumped it ("" otherwise). | ||||
| // Caller guarantees there is at least one bump (Proposed != "") in proposals. | ||||
| func applyRequiresUpdate(packageRoot string, proposals []requiresupdates.UpdateProposal, changelogEnabled bool, changelogType string) (newVersion string, err error) { | ||||
| manifestPath := filepath.Join(packageRoot, packages.PackageManifestFile) | ||||
| manifestBytes, err := os.ReadFile(manifestPath) | ||||
| if err != nil { | ||||
| return "", fmt.Errorf("reading manifest file failed: %w", err) | ||||
| } | ||||
|
|
||||
| manifestBytes, err = requiresupdates.Apply(manifestBytes, proposals) | ||||
| if err != nil { | ||||
| return "", err | ||||
| } | ||||
|
|
||||
| if changelogEnabled { | ||||
| manifestBytes, newVersion, err = requiresupdates.ApplyChangelog(packageRoot, manifestBytes, proposals, changelogType) | ||||
| if err != nil { | ||||
| return "", err | ||||
| } | ||||
| } | ||||
|
|
||||
| logger.Debugf("writing updated manifest: %s", manifestPath) | ||||
| if err := os.WriteFile(manifestPath, manifestBytes, 0o644); err != nil { | ||||
| return "", fmt.Errorf("writing manifest file failed: %w", err) | ||||
| } | ||||
| return newVersion, nil | ||||
| } | ||||
|
|
||||
| func printRequiresUpdateResult(result *requiresupdates.Result, w io.Writer, format string) error { | ||||
| if result == nil { | ||||
| return nil | ||||
|
|
||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no documentation added for these 2 new flags here:
elastic-package/cmd/requires.go
Lines 35 to 40 in 6158fcd