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.

Sign up for GitHub

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

Jump to bottom

Lrn 39610/feature/add signature v2 #50

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Lrn 39610/feature/add signature v2 #50

merged 1 commit into from
Jun 29, 2023

Conversation

markus-liang-lrn
Copy link
Contributor

No description provided.

@markus-liang-lrn markus-liang-lrn force-pushed the LRN-39610/feature/add-signature-v2 branch 7 times, most recently from 29bc520 to dcf31e7 Compare April 14, 2023 13:55
bhavya-shukla-lrn
bhavya-shukla-lrn reviewed Apr 17, 2023
View reviewed changes
.gitignore Outdated
@@ -2,3 +2,6 @@ target
Dist
.idea
learnositysdk.iml
.DS_Store
*/.DS_Store
docs/quickstart/assessment/webapps/
Copy link
Contributor

@bhavya-shukla-lrn bhavya-shukla-lrn Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Why is this added to git ignore "docs/quickstart/assessment/webapps/"?
add a new line in the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is for compiled folder

bhavya-shukla-lrn
bhavya-shukla-lrn reviewed Apr 17, 2023
View reviewed changes
docs/quickstart/assessment/src/main/java/com/learnosity/quickstart/QuestionsApp.java Outdated
}

Map<String, String> createSecurityObject(String domain) {
var secret = new HashMap();
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be called secret or security? I guess security is relevant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bhavya-shukla-lrn
bhavya-shukla-lrn reviewed Apr 17, 2023
View reviewed changes
docs/quickstart/assessment/src/main/webapp/ItemsApi.jsp Outdated
);
</script>
</body>
</html>
Copy link
Contributor

Choose a reason for hiding this comment

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

add a new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bhavya-shukla-lrn
bhavya-shukla-lrn reviewed Apr 17, 2023
View reviewed changes
docs/quickstart/assessment/src/main/webapp/QuestionsApi.jsp Outdated
);
</script>
</body>
</html>
Copy link
Contributor

Choose a reason for hiding this comment

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

add new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bhavya-shukla-lrn
bhavya-shukla-lrn reviewed Apr 17, 2023
View reviewed changes
docs/quickstart/assessment/src/main/webapp/ReportsApi.jsp Outdated
);
</script>
</body>
</html>
Copy link
Contributor

Choose a reason for hiding this comment

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

add new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bhavya-shukla-lrn
bhavya-shukla-lrn reviewed Apr 17, 2023
View reviewed changes
docs/quickstart/assessment/src/main/webapp/index.jsp Outdated
<li><a href="QuestionsApi.jsp">Questions API</a></li>
<li><a href="AuthorApi.jsp">Author API</a></li>
<li><a href="ReportsApi.jsp">Reports API</a></li>
</ul>
</body>
</html>
Copy link
Contributor

Choose a reason for hiding this comment

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

add new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bhavya-shukla-lrn
bhavya-shukla-lrn reviewed Apr 17, 2023
View reviewed changes
docs/quickstart/assessment/src/main/java/com/learnosity/quickstart/AuthorApp.java Outdated
}

Map<String, String> createSecurityObject(String domain) {
var secret = new HashMap();
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be security instead of secret?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bhavya-shukla-lrn
bhavya-shukla-lrn reviewed Apr 17, 2023
View reviewed changes
docs/quickstart/assessment/src/main/java/com/learnosity/quickstart/ItemsApp.java Outdated
}

Map<String, String> createSecurityObject(String domain) {
var secret = new HashMap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above ? security??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bhavya-shukla-lrn
bhavya-shukla-lrn reviewed Apr 17, 2023
View reviewed changes
docs/quickstart/assessment/src/main/webapp/AuthorApi.jsp Outdated
);
</script>
</body>
</html>
Copy link
Contributor

Choose a reason for hiding this comment

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

new line at end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bhavya-shukla-lrn
bhavya-shukla-lrn requested changes Apr 17, 2023
View reviewed changes
Copy link
Contributor

@bhavya-shukla-lrn bhavya-shukla-lrn left a comment

Choose a reason for hiding this comment

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

There are few questions and minor changes required

@markus-liang-lrn markus-liang-lrn force-pushed the LRN-39610/feature/add-signature-v2 branch from dcf31e7 to 39fef5d Compare April 17, 2023 20:52
TheRealEdDawson
TheRealEdDawson approved these changes Jun 22, 2023
View reviewed changes
Copy link
Contributor

@TheRealEdDawson TheRealEdDawson left a comment

Choose a reason for hiding this comment

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

I left a few minor comments, but generally, looks great, guys. Please go ahead!

TheRealEdDawson
TheRealEdDawson approved these changes Jun 23, 2023
View reviewed changes
Copy link
Contributor

@TheRealEdDawson TheRealEdDawson left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @bhavya-shukla-lrn . Please go ahead with it.

@bhavya-shukla-lrn bhavya-shukla-lrn force-pushed the LRN-39610/feature/add-signature-v2 branch 2 times, most recently from 19a8df5 to 548da56 Compare June 26, 2023 09:45
TheRealEdDawson
TheRealEdDawson approved these changes Jun 27, 2023
View reviewed changes
Copy link
Contributor

@TheRealEdDawson TheRealEdDawson left a comment

Choose a reason for hiding this comment

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

Looks good @markus-liang-lrn @bhavya-shukla-lrn @markkellyeire , I made some minor edits to text in README.md.

TheRealEdDawson
TheRealEdDawson reviewed Jun 27, 2023
View reviewed changes
CHANGELOG.md Outdated
@@ -6,6 +6,10 @@

## [Unreleased]

## [v0.16.4] - 2023-06-23
Copy link
Contributor

Choose a reason for hiding this comment

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

@markus-liang-lrn @bhavya-shukla-lrn @markkellyeire I think we can probably disable this Codacy rule ("Lists should be surrounded by blank lines"), since it goes against our own convention in this file?

@bhavya-shukla-lrn bhavya-shukla-lrn force-pushed the LRN-39610/feature/add-signature-v2 branch from 1d7e63d to 9230a52 Compare June 29, 2023 05:03
@bhavya-shukla-lrn bhavya-shukla-lrn force-pushed the LRN-39610/feature/add-signature-v2 branch from 9230a52 to 6129d4c Compare June 29, 2023 05:26
@bhavya-shukla-lrn bhavya-shukla-lrn force-pushed the LRN-39610/feature/add-signature-v2 branch from 6129d4c to 02f26d7 Compare June 29, 2023 05:30
@bhavya-shukla-lrn bhavya-shukla-lrn merged commit 9cf70d6 into master Jun 29, 2023
@bhavya-shukla-lrn bhavya-shukla-lrn deleted the LRN-39610/feature/add-signature-v2 branch June 29, 2023 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheRealEdDawson TheRealEdDawson TheRealEdDawson approved these changes

@bhavya-shukla-lrn bhavya-shukla-lrn bhavya-shukla-lrn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@markus-liang-lrn @TheRealEdDawson @bhavya-shukla-lrn