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

Add ssamadgh/ModelAssistant to suite #268

Merged
merged 5 commits into from Oct 29, 2018
Merged

Conversation

ssamadgh
Copy link
Contributor

@ssamadgh ssamadgh commented Oct 20, 2018

Pull Request Description

Added ModelAssistant Repository to Projects.json

Acceptance Criteria

To be accepted into the Swift source compatibility test suite, a project must:

  • be an Xcode or swift package manager project
  • support building on either Linux or macOS
  • target Linux, macOS, or iOS/tvOS/watchOS device
  • be contained in a publicly accessible git repository
  • maintain a project branch that builds against Swift 4.0 and passes any unit tests
  • have maintainers who will commit to resolve issues in a timely manner
  • be compatible with the latest GM/Beta versions of Xcode and swiftpm
  • add value not already included in the suite
  • be licensed with one of the following permissive licenses:
    • MIT
  • pass ./project_precommit_check script run

Ensure project meets all listed requirements before submitting a pull request.

@ssamadgh ssamadgh closed this Oct 23, 2018
@ssamadgh ssamadgh deleted the patch-2 branch October 23, 2018 16:52
@ssamadgh ssamadgh restored the patch-2 branch October 23, 2018 16:58
@ssamadgh ssamadgh reopened this Oct 23, 2018
@ssamadgh ssamadgh changed the title Added ModelAssistant Repository to Projects.json @swift-ci Please test source compatibility. Oct 23, 2018
@clackary
Copy link
Contributor

@ssamadgh Thanks for opening this! Allow me some time to review, and I'll get testing started when review is complete.

@clackary clackary changed the title @swift-ci Please test source compatibility. Add ssamadgh/ModelAssistant to suite Oct 23, 2018
Copy link
Contributor

@clackary clackary left a comment

Choose a reason for hiding this comment

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

I was unable to get ./project_precommit_check to pass with the current configuration. It failed because it couldn't find an Xcode workspace. This makes sense, because your project isn't configured as a workspace, but a regular Xcode project.

I've made a few changes locally, and was able to get it working by using a project configuration instead of workspace. These changes have been added as suggestions.

$ ./project_precommit_check ModelAssistant --earliest-compatible-swift-version 4.2
** CHECK **
--- Validating ModelAssistant Swift version 4.2 compatibility ---
--- Project configured to be compatible with Swift 4.2 ---
--- Checking ModelAssistant platform compatibility with Darwin ---
--- Platform compatibility check succeeded ---
--- Locating swiftc executable ---
$ xcrun -f swiftc
--- Checking installed Swift version ---
$ /Applications/Xcodes/PublicGMs/Xcode-10-GM.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swiftc --version
--- Version check succeeded ---
--- Executing build actions ---
$ ./runner.py --swift-branch swift-4.2-branch --swiftc /Applications/Xcodes/PublicGMs/Xcode-10-GM.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swiftc --projects projects.json --include-repos 'path == "ModelAssistant"' --include-versions 'version == "4.2"' --include-actions 'action.startswith("Build")'
PASS: ModelAssistant, 4.2, 1f1d37, ModelAssistant iOS, generic/platform=iOS
========================================
Action Summary:
     Passed: 1
     Failed: 0
    XFailed: 0
    UPassed: 0
      Total: 1
========================================
Repository Summary:
      Total: 1
========================================
Result: PASS
========================================
--- ModelAssistant checked successfully against Swift 4.2 ---

projects.json Outdated Show resolved Hide resolved
projects.json Outdated Show resolved Hide resolved
Zachary 'Clack' Cole and others added 2 commits October 23, 2018 23:50
Co-Authored-By: ssamadgh <ssamadgh@gmail.com>
Co-Authored-By: ssamadgh <ssamadgh@gmail.com>
@clackary
Copy link
Contributor

@swift-ci test

@ssamadgh ssamadgh changed the title Add ssamadgh/ModelAssistant to suite @swift-ci Please test source compatibility Oct 25, 2018
@ssamadgh
Copy link
Contributor Author

ssamadgh commented Oct 26, 2018

@clackary I made a few changes in the library, and I hope to be passed with the @swift-ci test. May I ask you test it again?
@swift-ci Please test source compatibility.

@clackary clackary changed the title @swift-ci Please test source compatibility Add ssamadgh/ModelAssistant to suite Oct 26, 2018
@clackary
Copy link
Contributor

@swift-ci test

@clackary
Copy link
Contributor

@ssamadgh Yes, I'd be happy to test again. The only way to start testing for this is for someone in the Swift Committers group to use @swift-ci test. Don't worry about updating the title.

Looking at the most recent failure, this may be a compiler crash. Let's see if it still happens after your changes to the library.

@clackary
Copy link
Contributor

Looks like this is crashing in the same way as before. I've filed SR-9102 to investigate. For the meantime, we can xfail this to be able to get it merged.

projects.json Outdated Show resolved Hide resolved
Copy link
Contributor

@clackary clackary left a comment

Choose a reason for hiding this comment

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

Please add xfail as suggested.

Co-Authored-By: ssamadgh <ssamadgh@users.noreply.github.com>
@ssamadgh
Copy link
Contributor Author

@clackary I committed the changes. @swift-ci test.

@clackary
Copy link
Contributor

@swift-ci test

@clackary
Copy link
Contributor

@ssamadgh Thanks for setting this up!

@clackary clackary merged commit 14ee4cf into apple:master Oct 29, 2018
@ssamadgh
Copy link
Contributor Author

ssamadgh commented Oct 29, 2018

@clackary Thanks for merging. What should I do if I updated my repository?
Is it enough to update the commit hash?

@clackary
Copy link
Contributor

Since you are the project maintainer, it's up to you how often you'd like to update the commit hash. Feel free to open a pull request with a new hash any time.

From what I've seen with other projects, this usually happens when updating to a new Swift version, or when there are large and significant changes to a project.

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.

None yet

2 participants