This repository was archived by the owner on Aug 1, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 180
Server personality #413
Merged
Merged
Server personality #413
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
7bf54c5
updates personality to []map[string]string where map has path and con…
edcbc31
Merge branch 'master' into personality
92e10b5
Encapsulate Personality. Encode contents for user
a2bfaea
use MarshalJSON
b11111a
Merge branch 'master' of https://github.com/rackspace/gophercloud int…
25d4569
Merge branch 'personality' of github.com:doubledutch/gophercloud
9546f89
Merge branch 'personality' of https://github.com/doubledutch/gophercl…
9748b7b
improves File documentation
60c1e89
adds personality to server created in acceptance tests
4d6c6e7
remove duplicate test from bad merge
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
What is the intended use case of the method? It seems like the base64 encoding process should go in the
ToServerCreateMap
method.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.
The file base64 encode process is used by
ToServerCreateMap
andToServerRebuildMap
. HavingFile
implementMarshalJSON
seemed like a good way to reduce putting the encode logic in two places and have aFile
know how to marshal itself.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.
Oh, I see; it's implementing the
Marshaler
interface. Can you add a comment aboveFile
mentioning that? Something like: "File implements thejson.Marshaler
interface, so when aCreate
orRebuild
operation is requested,json.Marshal
will callFile
'sMarshalJSON
method." The only reason for explicitness is because I don't think we have this anywhere else. Also, could you add aPersonality
field to the acceptance test that creates a server?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.
Sure! 👍