Skip to content

Conversation

SourangshuGhosh
Copy link
Contributor

Respected Sir
Very Sorry for a new pull request. This is a continuation of the previous issue #140 .My earkier forked repo wasn't working and some of the folders got deleted, so I have to fork again and do some work. As per your request I am attaching the photo. I have added the KOSS footer and logo but coudn't find out the grammatical mistake part you are saying about.
IMG_20200715_213721

@xypnox
Copy link
Member

xypnox commented Jul 15, 2020

Hi @SourangshuGhosh,

I really appreciate the enthusiasm with which you have attempted to fix the critical issue for the missing 404 page, and am excited to check out the changes. However, there are a few things that I would like to discuss first:

A commit represents a unique change logically.

Now, What do we consider as a unique logical change? For example, adding a new file with the contents is a unique change. But remember the change should be unique and a single unit in itself. For example, in this PR, the changes could be adding a 404.html page. And then another could be adding a .htaccess file.

Take care that saving a file once doesn't constitute a unique change in itself. There can be several changes to a piece of code, you are invited to go ahead and make them, but keep them local and do not commit every time you have made a slight change. We are patient and will wait for you to make the appropriate changes.

If you do not want us to review the code just now and are in progress of making the changes, change the type of the Pull request to "draft pull request". After you have made the changes that you want and are satisfied with the state of the PR, committed in logical block of changes, switch the type of Pull request from draft back to normal one so that it is ready for review.

Once again, I am glad that you have attempted to fix a critical bug and I will be patiently waiting for the PR to reach a status for review, till then please take care to commit a set of logical changes. Thanks!

PS: Also, it would be a good time to git squash to combine the several commits.

@SourangshuGhosh SourangshuGhosh marked this pull request as draft July 16, 2020 13:57
@SourangshuGhosh
Copy link
Contributor Author

I have changed the pull request to draft as you said to me. Can you give me some more suggestions regarding the changes I need to do in the fonts or style etc....

@rakaar rakaar marked this pull request as ready for review July 18, 2020 17:52
@SourangshuGhosh
Copy link
Contributor Author

@rakaar Is there improvement or suggestions regarding the changes I need to do in the fonts or style etc....

@rakaar
Copy link
Contributor

rakaar commented Jul 20, 2020

Thanks a lot for fixing the bug @SourangshuGhosh
Please don't forget these 2 important things

  1. Never send a PR from master branch, it is advisable to send from a branch with an appropriate name related to the task
  2. Squash your commits, when there are too many

Regarding (2) I am squashing and merging it myself

@rakaar rakaar merged commit 05dd855 into kossiitkgp:master Jul 20, 2020
@rakaar rakaar mentioned this pull request Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants