-
Notifications
You must be signed in to change notification settings - Fork 170
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
Add support for Contentful CLI #343
Conversation
I have some problems with the test functions. I will fix them soon. |
Corresponding blog post: https://rajapri28613.hashnode.dev/1password-shell-plugin-for-contentful |
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.
Thanks for the contribution 💪 Looks great, only a few comments and questions!
type Config struct { | ||
ManagementToken string `json:"managementToken"` | ||
ActiveEnvironmentId string `json:"activeEnvironmentId"` | ||
Host string `json:"host"` |
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.
Is the host something we could import from the json config file as well?
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.
Similar question for ActiveEnvironmentId
.
Importer: importer.TryAll( | ||
TryContentfulConfigFile(), | ||
)} |
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.
Let's use TryContentfulConfigFile()
directly, no need for TryAll
if there is only one importer.
ActiveEnvironmentId: "master", | ||
Host: "api.contentful.com", | ||
} | ||
contents, err := json.MarshalIndent(&config, "", " ") |
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.
No need for indentation, since this only produces a file meant to be consumed in a programmatic fashion. 😄
func TestPersonalAccessTokenProvisioner(t *testing.T) { | ||
plugintest.TestProvisioner(t, PersonalAccessToken().DefaultProvisioner, map[string]plugintest.ProvisionCase{ | ||
"default": { | ||
ItemFields: map[sdk.FieldName]string{ // TODO: Check if this is correct |
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.
Should this TODO
still be here?
needsauth.NotForHelpOrVersion(), | ||
needsauth.NotWithoutArgs(), |
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.
login
and logout
shouldn't authenticate either, I assume.
Hey @rajapri28613 , I see there hasn't been any activity on this PR in a bit, is there anything that we can help you with? We'd love to get this to the finish line together! 🚀 |
Hey @rajapri28613 , reaching out here again! Would love to see this get to the finish line. |
@rajapri28613 I am going to close this due to inactivity. Please do let us know if you'd like to pick this back up, would love to see this to the finish line! |
Overview
Add support for Contentful CLI. The shell plugin provisions a configuration file at
~/.contentfulrc.json
. The shell plugin also imports the Contentful token from the same file at the same path, if it exists.Type of change
Related Issue(s)
How To Test
contentful
should provision the token at~/.contentfulrc.json
and use it for authentication.Changelog
Add support for Contentful CLI.
Additional information