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

Demo PR #33

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Demo PR #33

wants to merge 3 commits into from

Conversation

samatcodeapprove
Copy link
Contributor

@samatcodeapprove samatcodeapprove commented Apr 12, 2023

In this PR I've added some new files:

  • file1.ts
  • file2.ts

I've also modified this file:

  • utils.ts

Review on CodeApprove

Copy link
Contributor Author

Automated comment from CodeApprove ➜

@alicethecoder please review this Pull Request

Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

⏳ Approval Pending (1 unresolved comments)
Approval will be granted automatically when all comments are resolved

LGTM, just one thing.


In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

Can you add a method comment?


👀 @samatcodeapprove it's your turn please take a look

Copy link
Contributor Author

@samatcodeapprove samatcodeapprove left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

Will do!

alicethecoder
alicethecoder previously approved these changes Apr 12, 2023
Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved: I have approved this change on CodeApprove and all of my comments have been resolved.

@alicethecoder alicethecoder dismissed their stale review April 12, 2023 04:26

Reviewer removed on CodeApprove

Copy link
Contributor Author

Automated comment from CodeApprove ➜

@alicethecoder please review this Pull Request

Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

⏳ Approval Pending (1 unresolved comments)
Approval will be granted automatically when all comments are resolved


In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

Have you tested this?


👀 @samatcodeapprove it's your turn please take a look

Copy link
Contributor Author

@samatcodeapprove samatcodeapprove left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

Yes I have!

alicethecoder
alicethecoder previously approved these changes Apr 12, 2023
Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved: I have approved this change on CodeApprove and all of my comments have been resolved.

@alicethecoder alicethecoder dismissed their stale review April 12, 2023 04:41

Reviewer removed on CodeApprove

Copy link
Contributor Author

Automated comment from CodeApprove ➜

@alicethecoder please review this Pull Request

Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

⏳ Approval Pending (1 unresolved comments)
Approval will be granted automatically when all comments are resolved


In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

Have you tested this?


👀 @samatcodeapprove it's your turn please take a look

Copy link
Contributor Author

@samatcodeapprove samatcodeapprove left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

Yes I have!

alicethecoder
alicethecoder previously approved these changes Apr 12, 2023
Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved: I have approved this change on CodeApprove and all of my comments have been resolved.

@alicethecoder alicethecoder dismissed their stale review April 12, 2023 04:50

Reviewer removed on CodeApprove

Copy link
Contributor Author

Automated comment from CodeApprove ➜

@alicethecoder please review this Pull Request

Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

⏳ Approval Pending (1 unresolved comments)
Approval will be granted automatically when all comments are resolved


In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

Have you tested this?


👀 @samatcodeapprove it's your turn please take a look

Copy link
Contributor Author

@samatcodeapprove samatcodeapprove left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

Yes I have!

alicethecoder
alicethecoder previously approved these changes Apr 12, 2023
Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved: I have approved this change on CodeApprove and all of my comments have been resolved.

@alicethecoder alicethecoder dismissed their stale review April 12, 2023 04:59

Reviewer removed on CodeApprove

Copy link
Contributor Author

Automated comment from CodeApprove ➜

@alicethecoder please review this Pull Request

Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

⏳ Approval Pending (1 unresolved comments)
Approval will be granted automatically when all comments are resolved


In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

Have you tested this?


👀 @samatcodeapprove it's your turn please take a look

Copy link
Contributor Author

@samatcodeapprove samatcodeapprove left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

I have!

alicethecoder
alicethecoder previously approved these changes Apr 12, 2023
Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved: I have approved this change on CodeApprove and all of my comments have been resolved.

@alicethecoder alicethecoder dismissed their stale review April 12, 2023 05:06

Reviewer removed on CodeApprove

Copy link
Contributor Author

@samatcodeapprove samatcodeapprove left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: demo/file1.ts:

> Line 1
export function generateRandomNumber(min: number, max: number): number {

This needs to change


In: demo/file1.ts:

> Line 9
  constructor(name: string, age: number) {

I love this!

Copy link
Contributor Author

Automated comment from CodeApprove ➜

@alicethecoder please review this Pull Request

Copy link

codeapprove bot commented Dec 6, 2023

Automated comment from CodeApprove ➜

👀 @samatcodeapprove it's your turn, please take a look

@alicethecoder alicethecoder dismissed their stale review December 6, 2023 01:39

Reviewer removed on CodeApprove

Copy link
Contributor Author

Automated comment from CodeApprove ➜

@alicethecoder please review this Pull Request

Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

⏳ Approval Pending (2 unresolved comments)
Approval will be granted automatically when all comments are resolved

This looks great, just one thing!


In: Discussion
This is a discussion on the whole PR


In: demo/file1.ts:

> Line 5
export class Person {

Could this have a comment?


In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

This looks great


👀 @samatcodeapprove it's your turn please take a look

Copy link
Contributor Author

@samatcodeapprove samatcodeapprove left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Thanks for the review!


In: Discussion
Thanks for discussing


In: demo/file1.ts:

> Line 5
export class Person {

Actually comments are impossible

alicethecoder
alicethecoder previously approved these changes Dec 6, 2023
Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved: I have approved this change on CodeApprove and all of my comments have been resolved.

@alicethecoder alicethecoder dismissed stale reviews from themself December 6, 2023 01:48

Review approved on CodeApprove

Copy link
Contributor Author

Automated comment from CodeApprove ➜

@alicethecoder please review this Pull Request

Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

⏳ Approval Pending (2 unresolved comments)
Approval will be granted automatically when all comments are resolved

I think this is close to done.


In: demo/file1.ts:

> Line 5
export class Person {

Can you add a comment to this class?


In: demo/utils.ts:

> Line 5
export function multiply(num1: number, num2: number): number {

This looks great.


👀 @samatcodeapprove it's your turn please take a look

Copy link
Contributor Author

@samatcodeapprove samatcodeapprove left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Thanks!


In: Discussion
Thanks for letting me know!


In: demo/file1.ts:

> Line 5
export class Person {

I'd like to do that later, I'm recording a demo now.

@alicethecoder alicethecoder dismissed their stale review December 6, 2023 02:00

Review approved on CodeApprove

Copy link
Collaborator

@alicethecoder alicethecoder left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved: I have approved this change on CodeApprove and all of my comments have been resolved.

Copy link
Contributor Author

Automated comment from CodeApprove ➜

👀 @alicethecoder it's your turn, please take a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants