buildasaurs / XcodeServerSDK

Access Xcode Server API with native Swift objects.
MIT License
399 stars 30 forks source link

Commits request #76

Closed cojoj closed 9 years ago

cojoj commented 9 years ago

I'm working on another point from #40 and I want to consult some matters before implementing anything.

It's obvious that List the commits included in an integration will consist of Commit model but... Response JSON is strange in some way and I'd like to clarify naming convention. Basically integration/id/commits response JSON looks like:

{
    "count": 1,
    "results": [{
        "_id": "62395864c6d0b1fc6536c0ab3a0d2b2d",
        "_rev": "3-ced1cebb3b420512ee66e7a1927e32f2",
        "commits": {
                  []
                },
                "integration": "62395864c6d0b1fc6536c0ab3a0d0733",
        "botID": "38a6d8e59f34d59c810e98f5c6030e02",
        "botTinyID": "23530A6",
        "endedTimeDate": [2015, 7, 7, 14, 15, 20],
        "doc_type": "commit",
        "tinyID": "127FDC7"
    }]
}

IMHO, main object should be called something like IntegrationCommits because there's Commits array inside of it but Commit, itself, doesn't containt fields to be XcodeServerEntity. What's more, commits array contains an array of repositories which holds actual commits - so another model? Actual Commit looks like:

{
    "XCSCommitCommitChangeFilePaths": [{
        "status": 4,
        "filePath": "cells/NewProductCell.m"
    }, {
        "status": 4,
        "filePath": "GreetingTextViewController.m"
    }, {
        "status": 4,
        "filePath": "ViewControllers/NewChooseProductViewController.m"
    }],
    "XCSCommitMessage": "Fixed ios7 bug",
    "XCSBlueprintRepositoryID": "715A78DA7313BF688CEE0585F44D4F0B2EAEE4B8",
    "XCSCommitContributor": {
        "XCSContributorEmails": ["test@example.com"],
        "XCSContributorName": "Foo Bar",
        "XCSContributorDisplayName": "Foo Bar"
    },
    "XCSCommitHash": "aeb85aed37e633b6c9776cb863a39f922cb464d9",
    "XCSCommitTimestamp": "2015-07-07T14:14:29.000Z",
    "XCSCommitTimestampDate": [2015, 7, 7, 14, 14, 29]
}

I have a feeling I'm complicating everything but what do you think - should those models be separated or treated like one big, nested model and inherit from XcodeServerEntity?

czechboy0 commented 9 years ago

Honestly, we're not losing anything by having them inherit from XcodeServerEntity. We get a couple of nice parsing behaviors this way. So I'd suggest making all of our objects inherit from XcodeServerEntity, even Repository (which it isn't at the moment, but I think it should be now).

Yeah, IntegrationCommits sounds like a good model object. Commit, will be another one. Then, Contributor might be another one.

Not sure about the list of repositories, can you paste that here as well?

cojoj commented 9 years ago

I don't have this JSON fragment as all of my Bots are configured to have only one repository but from the naming you can guess that commits can contain an array. In case of one configured repository it looks like:

{
  "commits": {
    "715A78DA7313BF688CEE0585F44D4F0B2EAEE4B8": {
      "XCSCommitCommitChangeFilePaths": [
        {
          "status": 4,
          "filePath": "cells/NewProductCell.m"
        },
        {
          "status": 4,
          "filePath": "GreetingTextViewController.m"
        },
        {
          "status": 4,
          "filePath": "ViewControllers/NewChooseProductViewController.m"
        }
      ],
      "XCSCommitMessage": "Fixed ios7 bug",
      "XCSBlueprintRepositoryID": "715A78DA7313BF688CEE0585F44D4F0B2EAEE4B8",
      "XCSCommitContributor": {
        "XCSContributorEmails": [
          "test@example.com"
        ],
        "XCSContributorName": "Foo Bar",
        "XCSContributorDisplayName": "Foo Bar"
      },
      "XCSCommitHash": "aeb85aed37e633b6c9776cb863a39f922cb464d9",
      "XCSCommitTimestamp": "2015-07-07T14:14:29.000Z",
      "XCSCommitTimestampDate": [2015, 7, 7, 14, 14, 29]
    }
  }
}

So, "715A78DA7313BF688CEE0585F44D4F0B2EAEE4B8" is a XCSBlueprintRepositoryID and I think it's logical that it can turn to be an array but I'm not 100% sure...

czechboy0 commented 9 years ago

Hmm I doubt that. As you can see in XCSContributorEmails, for example, even with one entry it's still an array. And this makes sense - an integration runs only on one repository. And XCSBlueprintRepositoryID gives you the id of the blueprint of the repo that ran. So I think we're fine just having it as a String.

cojoj commented 9 years ago

👍

In that case IntegrationCommits will have property of type [Commits], right?

czechboy0 commented 9 years ago

Hmm only now I noticed that commits are a dictionary, not an array. Could you please post the whole output, both parts above in one JSON? Thanks

cojoj commented 9 years ago

I'll do it later and I'll try to prove many repositories scenario, so it'll turn into array of dictionaries.

Give me time, I need some 🍖🍗🍲

cojoj commented 9 years ago

I've checked multi-repository thing and it looks like you're right @czechboy0 - there will be only one repo and this view proves it: zrzut ekranu 2015-07-20 o 21 52 07


Regarding whole repository response JSON, here you go:

{
    "count": 1,
    "results": [{
        "_id": "272167d4affaf38ea93c6e403d001c60",
        "_rev": "3-d856e17d2e88237b37c190591ddaaa48",
        "commits": {
            "A36AEFA3F9FF1F738E92F0C497C14977DCE02B97": [{
                "XCSCommitCommitChangeFilePaths": [{
                    "status": 4,
                    "filePath": "Podfile.lock"
                }],
                "XCSCommitMessage": "new pods\n",
                "XCSBlueprintRepositoryID": "A36AEFA3F9FF1F738E92F0C497C14977DCE02B97",
                "XCSCommitContributor": {
                    "XCSContributorEmails": ["honza@swiftkey.com"],
                    "XCSContributorName": "Honza Dvorsky",
                    "XCSContributorDisplayName": "Honza Dvorsky"
                },
                "XCSCommitHash": "deeca3728ed42406ee34e595feff4ed3bb78efb4",
                "XCSCommitTimestamp": "2015-07-20T13:36:04.000Z",
                "XCSCommitTimestampDate": [2015, 7, 20, 13, 36, 4, 0]
            }, {
                "XCSCommitCommitChangeFilePaths": [{
                    "status": 4,
                    "filePath": "README.md"
                }],
                "XCSCommitMessage": "more emoji",
                "XCSBlueprintRepositoryID": "A36AEFA3F9FF1F738E92F0C497C14977DCE02B97",
                "XCSCommitContributor": {
                    "XCSContributorEmails": ["honza@swiftkey.com"],
                    "XCSContributorName": "Honza Dvorsky",
                    "XCSContributorDisplayName": "Honza Dvorsky"
                },
                "XCSCommitHash": "9f54f31ceffc89ddf735be250e911238631494aa",
                "XCSCommitTimestamp": "2015-07-19T10:23:10.000Z",
                "XCSCommitTimestampDate": [2015, 7, 19, 10, 23, 10, 0]
            }, {
                "XCSCommitCommitChangeFilePaths": [{
                    "status": 4,
                    "filePath": "CHANGELOG.md"
                }],
                "XCSCommitMessage": "changelog\n",
                "XCSBlueprintRepositoryID": "A36AEFA3F9FF1F738E92F0C497C14977DCE02B97",
                "XCSCommitContributor": {
                    "XCSContributorEmails": ["honza@swiftkey.com"],
                    "XCSContributorName": "Honza Dvorsky",
                    "XCSContributorDisplayName": "Honza Dvorsky"
                },
                "XCSCommitHash": "98438eee1e8d891b34c777b57485f1ac251e5b1b",
                "XCSCommitTimestamp": "2015-07-18T18:12:02.000Z",
                "XCSCommitTimestampDate": [2015, 7, 18, 18, 12, 2, 0]
            }, {
                "XCSCommitCommitChangeFilePaths": [],
                "XCSCommitMessage": "Merge pull request #72 from czechboy0/hd/cocoapods\n\nSwitching to CocoaPods",
                "XCSBlueprintRepositoryID": "A36AEFA3F9FF1F738E92F0C497C14977DCE02B97",
                "XCSCommitContributor": {
                    "XCSContributorEmails": ["honza@swiftkey.com"],
                    "XCSContributorName": "Honza Dvorsky",
                    "XCSContributorDisplayName": "Honza Dvorsky"
                },
                "XCSCommitHash": "43653e7e9dcefab8089c05641d4a013002df9ff6",
                "XCSCommitTimestamp": "2015-07-18T14:11:19.000Z",
                "XCSCommitTimestampDate": [2015, 7, 18, 14, 11, 19, 0]
            }, {
                "XCSCommitCommitChangeFilePaths": [{
                    "status": 4,
                    "filePath": "XcodeServerSDK.podspec"
                }],
                "XCSCommitMessage": "new proper version\n",
                "XCSBlueprintRepositoryID": "A36AEFA3F9FF1F738E92F0C497C14977DCE02B97",
                "XCSCommitContributor": {
                    "XCSContributorEmails": ["honza@swiftkey.com"],
                    "XCSContributorName": "Honza Dvorsky",
                    "XCSContributorDisplayName": "Honza Dvorsky"
                },
                "XCSCommitHash": "64526cb1dbe14b954c6edb080a9ae483b7fdfa2b",
                "XCSCommitTimestamp": "2015-07-18T14:10:16.000Z",
                "XCSCommitTimestampDate": [2015, 7, 18, 14, 10, 16, 0]
            }]
        },
        "integration": "272167d4affaf38ea93c6e403d000d3a",
        "botID": "272167d4affaf38ea93c6e403d0004c3",
        "botTinyID": "F977308",
        "endedTimeDate": [2015, 7, 20, 19, 36, 50, 237],
        "doc_type": "commit",
        "tinyID": "30C0AE6"
    }]
}
czechboy0 commented 9 years ago

Hmm my guess is if you have git submodules, the commits field (a dictionary) will map from each repository's id to its array of commits. So I guess we have to map our models the same way. Commit needs to be of type [String: [Commit]].

cojoj commented 9 years ago

IntegrationCommits will be a merge of Bot and Integration with Dictionary of commits... I'm looking for some logic here... 😏

czechboy0 commented 9 years ago

Just the Bot's id. It's all about the Commits in the end though.

cojoj commented 9 years ago

Nevermind... Sometimes I just need to bite my tounge before a click comment... 😶

I'll sit on this tomorrow and will provide a proposal model for this. Thanks for your help! Much appreciated 👍

czechboy0 commented 9 years ago

Anything else in this thread, @cojoj? Otherwise I'll close it.

cojoj commented 9 years ago

Sure, close it as for now we've move all of our conversation to #77 😉