这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@mnivet
Copy link
Collaborator

@mnivet mnivet commented Jan 16, 2019

Implements the Src resource https://developer.atlassian.com/bitbucket/api/2/reference/resource/repositories/%7Busername%7D/%7Brepo_slug%7D/src#get

Only GET operations have been implemented since the POST operation is really particular.
The POST allow to create commits. So why it's in Src and not in Commit resource ?
And it use multipart/form-data while our implementation use application/json to post data for API V2 in RequestExecutorV2 which means that it will require important changes and probably breaking changes to be able to specify how post operations should be performed.

public class TreeEntryLinks
{
public Link Self { get; set; }
public Link Meta { get; set; }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The meta property was not declared in the existing Links POCO.
Since TreeEntry responses returns only two links I've created a new POCO that represent the only two links that are returned in a TreeEntry, but we can also choose to add a Meta property in the existing Links class since there is already a lot of cases where we manipulate POCOs that won't be entirely filled.

What should we do in general ?

  • Have POCOs that cover all the possible properties in the various scenarios ?
  • Or have a lot of specialized POCOs that are tied to each scenario ?

In the first case, we will have less objects and potentially allow the consumers to easily complete and reuse POCOs without doing mapping between similar objects.
In the second case, we will have more objects, but consumers will be less tempted to think that they can found some data in some contexts. However it may happens that consumers will have to do some mappings between similar POCOs.

I asked because the question is the same for the Commit property of the TreeEntry POCO.
There, I've used an existing POCO, but in fact it's more a commit reference than a full commit POCO, since we only get the hash and the link that allow to get the full commit POCO.

So it would be better to have a same strategy for the Links property than for the Commit property and choose between reusing POCOs or declaring specialized POCOs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now I've tried to expose the facts I'll give my opinion:

I prefer the second case because like that we always know exactly what we consume, and because use cases where we have to perform some mappings should be rare. And even when mappings are required, by experience, we often want to extract just few fields and drop a lot of others.

Reusing POCOs has always appear to me as a lazy way since it spares you to find names for similar POCOs (like commit / CommitReference or Links / TreeEntryLinks) and it spares you to think of an inheritance model (like: does Commit POCO should inherit from Commit Reference ?)

But a lazy approach could also be a valid approach for a project where we don't want to invest too much time...

And you @MitjaBezensek, what's your opinion ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've found in the bitbucket documentation a paragraph about their point of view Condensed Versus Full Objects
So for them what I've called 'specialized POCO" or "Reference to another object" is called a "condensed representation"

I've also discovered that some specialized POCOs for condensed form of the objects have already been created:

  • ProjectInfo
  • PullRequestInfo
  • UserShort

So I think that the best solution is to continue with the "{type}Info" pattern, and document that clearly somewhere in our documentation that it's the pattern naming for the condensed form of the objects

Copy link
Owner

Choose a reason for hiding this comment

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

I completely agree with you and I'm all for specialized POCOs. I tried to use the Info pattern where I saw the need for it.

That way the end users know exactly what they are getting. Otherwise we might start getting support questions of the type "Why is this field empty when I call this?".

# Conflicts:
#	Coverage.md => coverage of Src resource has been rewritten with the new format of the document
@mnivet mnivet mentioned this pull request Jan 21, 2019
also transform the TreeEntry POCO to camel case since it's what is done in all other POCOs
and add the history link in the TreeEntryLinks
… on the content of the POCOs in multiple cases

This allow to bring the light on the differences between Files and Directories relative to how the TreeEntry POCO is filled
…ectory POCOs

Those have been designed to be more intuitive to consume since there is a clear separation of the properties that are accessible for files and directories
@mnivet mnivet merged commit c7ebbd9 into master Jan 23, 2019
@mnivet mnivet deleted the SrcResource branch January 23, 2019 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants