This blog post is for my Second Code review and Third Code Review. Because the second code review is very simple and short, I decided also to put the third code review.
After I finished working on the first code review on the telescope project. I decided to work on the IPC project for the next code review. I just went in a pull request list of the IPC project and just choose one issue which come into my sight first. and it was audit&fix information.md. And I started to read a code. and I saw that the deployment of the most current issue was failed. So, I decided to find the reason why it failed. because it was all passed in previous commits.
And I think the one who worked on this pull request had a conflict when he/she pushed the last commits. because I found a tag that git automatically generates if the conflict exists.
"execa": "^5.1.1",
"mr-pdf": "^1.0.7",
"wait-on": "^6.0.0"
=======
"docusaurus": "^1.14.7"
>>>>>>> issue-44
}
So, I just added a comment to erase these tags.
And the third pull request was also about the IPC web page. That was about audit and fix string-library.md file.
In this pull request, I found some issues in code snippets. I left two comments about the array size of the cstring.
The first one is about strcpy of char array.
int main(void)
{
char str[31], copy[21] = "?";
int i, len;
printf("Source : ");
scanf("%30[^\n]%*c", str);
len = strlen(str);
if (len < 21)
{
strcpy(copy, str);
printf("Copy : %s\n", copy);
}
As this code is going to copy str to copy. I think the size of the array str and size of array copy should be the same or the copy should be larger than str, even though the if statement blocks the code to copy less than 21.
I can understand that why did the writer write like that. But, it can make the student in confusion. Because I was a student who was confused because of an unclear code snippet and I need to go to the professor and ask why is it like this.
And next one was also about concatenating char array. I left a code review on this because I think it will be better if the given name is there instead of the full name.
int main(void)
{
int i, len;
char surname[31], fullName[62];
printf("Given name : ");
scanf("%30[^\n]%*c", fullName);
printf("Surname : ");
scanf("%30[^\n]%*c", surname);
strcat(fullName, " ");
strcat(fullName, surname);
printf("Full name is %s\n", fullName);
return 0;
}
But it was my mistake that I understand that the code want to add surname and full name array to make it surname + full name. But after I left a comment on this code review, I got a reply on it and I noticed that I was wrong.
The reply was
I don't think so. This is the example for
strcat() which concatenates surname and
givenname to fullname so the fullname
arr's size should always be greater than
the surname arr's size.
For the second code review, I learned that git conflict tags can cause a fail in deployment. And for the third code review, I learned that I really need to understand well before I leave a comment on an issue. or else, I might make the PR creator confused.
Top comments (0)