-
Notifications
You must be signed in to change notification settings - Fork 4
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
PM1.3 Team 3 #11
PM1.3 Team 3 #11
Conversation
sendGet(..) method should later be merged into an utility package containing all HTTP requests
RT client for create issues
Github Json Parser
…trieve from Foot Prints v12
Issue 7/refactoring
#7 [Update] Adding the initial operations and services that we can re…
#10 [Update] Bash script to compile all java files
Thanks, I will review and comment as I go along until I mark as REVIEW DONE. |
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.
Initial pass; more to follow when I get back to it.
* Usage: https://bitbucket.org/soen487-w18-03/soen487-w18-team03/issues/8/github-json-parser | ||
*/ | ||
|
||
package fp.v11; |
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.
(A) This would naturally be github.
instead of fp.v11
.
import javax.json.JsonString; | ||
|
||
|
||
public class GitIssueDetails { |
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.
(B) GitHub
is a better class name
@@ -0,0 +1,90 @@ | |||
package fp.v11; |
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.
(C) naturally, should be package rt;
(D) |
(E) for your (f) |
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.
Initial REVIEW DONE
@@ -1,3 +1,5 @@ | |||
# Ignore patterns | |||
|
|||
*~ | |||
|
|||
.idea/ |
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.
(F) why IntellIJ build system was removed?
* Gets issue details | ||
* | ||
*/ | ||
public void getIssueDetails(); |
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.
(G) None of these should return void
. It would be some kind of collection; for example a hashtable or a vector depending on the call. See what Perl is returning.
* Links two issues | ||
* | ||
*/ | ||
public void linkIssues(); |
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.
(H) no void
as well
|
||
private final Console console; | ||
private final Scanner scanner; | ||
private final String USERNAME_INPUT_TEXT = "Your username: "; |
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.
(I) you provider is not a singleton, so constants like that should be static
...
* Describes the services offered by | ||
* FootPrint version 12 | ||
*/ | ||
public interface FootPrints12Interface { |
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.
(J) as you probably realize this should be ISplints
... or an ISplints
mapping to the API of FP12. Like what's an item? You can keep FootPrints12Interface
for now then we need also ISplints
here and an adapter implementing both.
/** | ||
* CONTACT OPERATIONS | ||
*/ | ||
void createContract(); |
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.
(K) this and similar others probably mean ..Contact..
, not ..Contract..
@@ -0,0 +1,12 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> | |||
<module type="JAVA_MODULE" version="4"> |
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.
(L) I gather this belongs to a partial IntelliJ build system. It should not be in src/
; recall your "src" is really under src/java/
Merged to easier ublock PM2 contribs; need to re-check (A)-(L) TODOs, later. |
Contributes to #6. |
@qtdnguyen -- JFYI of what's coming |
Tasks
( The current version has been tagged as
pm1
and submitted to EAS as .zip)c) Refactor existing Java SOAP API per Perl’s structure
See
splints/src/java/fp/v11/Splints/
for more detailsd) Provide a simple JSON reader parser client (GET) for GitHub issues
[Updated] GitIssueDetails.java usage:
Go to lib/java folder
cd lib/java
Compile GitIssueDetails.java using javax.json-1.0.jar as class path
javac -classpath .:./javax.json-1.0.jar ../../src/java/fp/v11/github/GitHub.java
Go to src/java folder
cd ../../src/java/fp/v11
Run GitIssueDetails class using javax.json-1.0.jar as class path
java -classpath .:../../../../lib/java/javax.json-1.0.jar github/GitHub
e) Add a simple client to call RT create issues
RtCreateIssue.java usage:
f) Bonus: enable CI build and install of a test RT system and auto-test your methods above.
Compilation test usage:
For more details
g) Bonus: Provide an equivalent Java API client for v12:
The current implementation of this feature involves establishing a Java Interface that uses the operations and services provided by FootPrints in their version 12. This interface can be found under
v12/FootPrints12Interface.java
For more details