Skip to content
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

feat: changes to support motd #385

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

feat: changes to support motd #385

wants to merge 8 commits into from

Conversation

tonystarkjr3
Copy link
Collaborator

@tonystarkjr3 tonystarkjr3 commented Oct 9, 2023

Summary

PR contains code to support retrieval of and recordkeeping of update frequency for a "message of the day" similarly to how it works in the core CLI program. There are some assumptions about replicating the entries for MotD that appear in the core program's configuration in plugin configuration files (adding a key "MessageOfTheDayTime" and one key for the location on COS from where the MotD payload can be fetched); this may change later in the review process.

@tonystarkjr3 tonystarkjr3 added the do-not-merge Label to remind reviewers NOT merge this PR label Oct 9, 2023
@@ -9,6 +9,8 @@ import (
"github.com/IBM-Cloud/ibm-cloud-cli-sdk/bluemix"
"github.com/IBM-Cloud/ibm-cloud-cli-sdk/bluemix/configuration"
"github.com/IBM-Cloud/ibm-cloud-cli-sdk/bluemix/models"

// "github.com/IBM-Cloud/ibm-cloud-cli-sdk/plugin"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Remove comment

go.mod Outdated
@@ -1,19 +1,20 @@
module github.com/IBM-Cloud/ibm-cloud-cli-sdk

go 1.20
go 1.19
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let move the go version to match what is on the core CLI

return false
}

func CheckMessageOfTheDay(client *rest.Client, config core_config.ReadWriter, pluginConfig plugin.PluginConfig, modURL string, ui terminal.UI, version string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The version parameter is never used in this method. Lets remove it.

bluemix/motd/motd.go Outdated Show resolved Hide resolved
@tonystarkjr3
Copy link
Collaborator Author

So sorry about the delay on this response @Aerex . I think you made these comments before I left for end-of-year vacation, and I was oblivious to them until the issue became re-ranked to a higher priority just recently. I made straightforward corrections to 3/4 of your comments and asked a followup about the last one.

Anupam Pokharel added 2 commits July 11, 2024 09:31
@tonystarkjr3 tonystarkjr3 requested a review from Aerex July 12, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge Label to remind reviewers NOT merge this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants