Skip to content

Commit

Permalink
Address PR review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
MDrakos committed Oct 28, 2024
1 parent 33bf8fb commit 34d3e1a
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 7 deletions.
8 changes: 3 additions & 5 deletions internal/runners/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ func (i *Install) Run(params Params) (rerr error) {
var oldCommit *bpModel.Commit
var reqs requirements
var ts time.Time
var override bool
{
pg = output.StartSpinner(out, locale.T("progress_search"), constants.TerminalAnimationInterval)

Expand All @@ -134,7 +133,6 @@ func (i *Install) Run(params Params) (rerr error) {
if err != nil {
return errs.Wrap(err, "Unable to get timestamp from params")
}
override = params.Timestamp.String() != ""

// Get languages used in current project
languages, err := model.FetchLanguagesForBuildScript(oldCommit.BuildScript())
Expand All @@ -155,7 +153,7 @@ func (i *Install) Run(params Params) (rerr error) {

// Prepare updated buildscript
script := oldCommit.BuildScript()
if err := prepareBuildScript(script, reqs, ts, override); err != nil {
if err := prepareBuildScript(script, reqs, ts); err != nil {
return errs.Wrap(err, "Could not prepare build script")
}

Expand Down Expand Up @@ -342,8 +340,8 @@ func (i *Install) renderUserFacing(reqs requirements) {
i.prime.Output().Notice("")
}

func prepareBuildScript(script *buildscript.BuildScript, requirements requirements, ts time.Time, override bool) error {
script.SetAtTime(ts, override)
func prepareBuildScript(script *buildscript.BuildScript, requirements requirements, ts time.Time) error {
script.SetAtTime(ts, true)
for _, req := range requirements {
requirement := types.Requirement{
Namespace: req.Resolved.Namespace,
Expand Down
2 changes: 1 addition & 1 deletion internal/runners/platforms/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (a *Add) Run(params AddRunParams) (rerr error) {

// Prepare updated buildscript
script := oldCommit.BuildScript()
script.SetAtTime(ts, false)
script.SetAtTime(ts, true)
script.AddPlatform(*platform.PlatformID)

// Update local checkout and source runtime changes
Expand Down
2 changes: 1 addition & 1 deletion internal/runners/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (u *Upgrade) Run(params *Params) (rerr error) {
if err != nil {
return errs.Wrap(err, "Failed to fetch latest timestamp")
}
bumpedBS.SetAtTime(ts, params.Timestamp.String() != "")
bumpedBS.SetAtTime(ts, true)

// Since our platform is commit based we need to create a commit for the "after" buildplan, even though we may not
// end up using it it the user doesn't confirm the upgrade.
Expand Down
3 changes: 3 additions & 0 deletions pkg/buildscript/buildscript.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ func (b *BuildScript) AtTime() *time.Time {
return b.atTime
}

// SetAtTime sets the AtTime value, if the buildscript already has an AtTime value
// and `override=false` then the value passed here will be discarded.
// Override should in most cases only be true if we are making changes to the buildscript.
func (b *BuildScript) SetAtTime(t time.Time, override bool) {
if b.atTime != nil && !override {
return
Expand Down

0 comments on commit 34d3e1a

Please sign in to comment.