You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@age.apache.org by "FahadZaheerfzr (via GitHub)" <gi...@apache.org> on 2023/01/21 10:42:06 UTC
[GitHub] [age] FahadZaheerfzr opened a new pull request, #585: Age driver swift
FahadZaheerfzr opened a new pull request, #585:
URL: https://github.com/apache/age/pull/585
This pull request contains the initial connection of PostgreSQL in Swift
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] FahadZaheerfzr commented on a diff in pull request #585: Age driver swift
Posted by "FahadZaheerfzr (via GitHub)" <gi...@apache.org>.
FahadZaheerfzr commented on code in PR #585:
URL: https://github.com/apache/age/pull/585#discussion_r1083334182
##########
drivers/age-swift/Tests/age-swiftTests/age_swiftTests.swift:
##########
@@ -0,0 +1,24 @@
+import XCTest
+@testable import age_swift
+
+let host = "127.0.0.1";
+let port = 5432 ;
+let user = "root";
+let db_name = "demo";
+
+final class age_swiftTests: XCTestCase {
+ func testExample() throws {
+ // This is an example of a functional test case.
+ // Use XCTAssert and related functions to verify your tests produce the correct
+ // results.
+ let age = Age()
+
+ let connection_param:[String:Any] = ["host":host,"port":port, "user":user, "dbname":db_name]
Review Comment:
Makes sense. Will try to do this too in my upcoming pull request
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on a diff in pull request #585: Age driver swift
Posted by "jasperblues (via GitHub)" <gi...@apache.org>.
jasperblues commented on code in PR #585:
URL: https://github.com/apache/age/pull/585#discussion_r1083334116
##########
drivers/age-swift/README.md:
##########
@@ -0,0 +1,3 @@
+# age-swift
Review Comment:
Once the readme is set up, I will comment on the usage and perhaps have some suggestions for the correct level of abstraction for a driver - it should be easy to use in the most common scenarios, possible in the edge cases.
Great work so far!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] FahadZaheerfzr commented on a diff in pull request #585: Age driver swift
Posted by "FahadZaheerfzr (via GitHub)" <gi...@apache.org>.
FahadZaheerfzr commented on code in PR #585:
URL: https://github.com/apache/age/pull/585#discussion_r1083334137
##########
drivers/age-swift/Tests/age-swiftTests/age_swiftTests.swift:
##########
@@ -0,0 +1,24 @@
+import XCTest
+@testable import age_swift
+
+let host = "127.0.0.1";
+let port = 5432 ;
+let user = "root";
+let db_name = "demo";
+
+final class age_swiftTests: XCTestCase {
+ func testExample() throws {
Review Comment:
Yes will keep adding new test cases, in my upcoming pull requests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] FahadZaheerfzr commented on a diff in pull request #585: Age driver swift
Posted by "FahadZaheerfzr (via GitHub)" <gi...@apache.org>.
FahadZaheerfzr commented on code in PR #585:
URL: https://github.com/apache/age/pull/585#discussion_r1083333984
##########
drivers/age-swift/README.md:
##########
@@ -0,0 +1,3 @@
+# age-swift
Review Comment:
Alright I will do that in my next pull request
##########
drivers/age-swift/Sources/age-swift/age.swift:
##########
@@ -0,0 +1,108 @@
+//
Review Comment:
Will add this too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] dehowef commented on pull request #585: Age driver swift
Posted by "dehowef (via GitHub)" <gi...@apache.org>.
dehowef commented on PR #585:
URL: https://github.com/apache/age/pull/585#issuecomment-1418596463
Hey Fahad, thank you for your contributions. I think something that would be helpful is if you squash your commits down. This way your pull request will be more concise with what you are adding to the project. Also, adding a short description detailing the changes under your commit for what changes were added is helpful for helping other contributors to know what you are adding with each commit. You can reference commits done on the main branches for example. Thank you !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on a diff in pull request #585: Age driver swift
Posted by "jasperblues (via GitHub)" <gi...@apache.org>.
jasperblues commented on code in PR #585:
URL: https://github.com/apache/age/pull/585#discussion_r1083333904
##########
drivers/age-swift/Tests/age-swiftTests/age_swiftTests.swift:
##########
@@ -0,0 +1,24 @@
+import XCTest
+@testable import age_swift
+
+let host = "127.0.0.1";
+let port = 5432 ;
+let user = "root";
+let db_name = "demo";
+
+final class age_swiftTests: XCTestCase {
+ func testExample() throws {
+ // This is an example of a functional test case.
+ // Use XCTAssert and related functions to verify your tests produce the correct
+ // results.
+ let age = Age()
+
+ let connection_param:[String:Any] = ["host":host,"port":port, "user":user, "dbname":db_name]
Review Comment:
How about having a type to represent the connection params?
So that the IDE (Xcode) can code-complete these? Rather than the user having to remember the name of strings, and perhaps type them incorrectly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on a diff in pull request #585: Age driver swift
Posted by "jasperblues (via GitHub)" <gi...@apache.org>.
jasperblues commented on code in PR #585:
URL: https://github.com/apache/age/pull/585#discussion_r1083333842
##########
drivers/age-swift/Tests/age-swiftTests/age_swiftTests.swift:
##########
@@ -0,0 +1,24 @@
+import XCTest
+@testable import age_swift
+
+let host = "127.0.0.1";
+let port = 5432 ;
+let user = "root";
+let db_name = "demo";
+
+final class age_swiftTests: XCTestCase {
+ func testExample() throws {
Review Comment:
Suggest to name the test method as following:
```scenarioName_should_expectedOutcome()```
For example:
```connectingWithSuppliedParams_shouldNotError()```
Suggest to also add some more scenarios:
* What happens when the incorrect params are supplied? What's the expected error message? Ensure it does not crash.
* What happens when we execute CYPHER statements returning vertices?
* What happens when we execute CYPHER statements returning vertices and edges?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on a diff in pull request #585: Age driver swift
Posted by "jasperblues (via GitHub)" <gi...@apache.org>.
jasperblues commented on code in PR #585:
URL: https://github.com/apache/age/pull/585#discussion_r1083333430
##########
drivers/age-swift/README.md:
##########
@@ -0,0 +1,3 @@
+# age-swift
Review Comment:
Perhaps you can add a quick start guide (Swift usage), with links to AGE quick start (for setting up db).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on a diff in pull request #585: Age driver swift
Posted by "jasperblues (via GitHub)" <gi...@apache.org>.
jasperblues commented on code in PR #585:
URL: https://github.com/apache/age/pull/585#discussion_r1083334207
##########
drivers/age-swift/Tests/age-swiftTests/age_swiftTests.swift:
##########
@@ -0,0 +1,24 @@
+import XCTest
+@testable import age_swift
+
+let host = "127.0.0.1";
+let port = 5432 ;
+let user = "root";
+let db_name = "demo";
+
+final class age_swiftTests: XCTestCase {
+ func testExample() throws {
Review Comment:
You can add more commits to this PR. They can be squashed down when merging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on a diff in pull request #585: Age driver swift
Posted by "jasperblues (via GitHub)" <gi...@apache.org>.
jasperblues commented on code in PR #585:
URL: https://github.com/apache/age/pull/585#discussion_r1083333460
##########
drivers/age-swift/Sources/age-swift/age.swift:
##########
@@ -0,0 +1,108 @@
+//
Review Comment:
Is there a standard AGE copyright header, including Apache license details?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] jasperblues commented on pull request #585: Age driver swift
Posted by "jasperblues (via GitHub)" <gi...@apache.org>.
jasperblues commented on PR #585:
URL: https://github.com/apache/age/pull/585#issuecomment-1418641335
<img width="437" alt="image" src="https://user-images.githubusercontent.com/430321/216912799-e174520c-4565-4d26-9029-e07f65b5899a.png">
@dehowef With Github its also possible to squash while merging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [age] FahadZaheerfzr closed pull request #585: Basic Apache Age connection in Swift
Posted by "FahadZaheerfzr (via GitHub)" <gi...@apache.org>.
FahadZaheerfzr closed pull request #585: Basic Apache Age connection in Swift
URL: https://github.com/apache/age/pull/585
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org