phpreboot / website

phpreboot website
9 stars 23 forks source link

Scrutinizer-ci rating went down #63

Open kapilsharma opened 8 years ago

kapilsharma commented 8 years ago

https://scrutinizer-ci.com/g/phpreboot/website/

Rating started with 10. However with recent commits, it went down to 8.44.

Need to fix issues reported by scrutinizer and get rating back to at least 9.5

mhetreramesh commented 8 years ago

Started working on improving scrutinizer rating in the branch hotfix/1.0.1

kapilsharma commented 8 years ago

Hi Ramesh,

What is the status on this issue. I believe many violations are on artisan commands so you must be editing them. However I need to update commands for issue#76.

If you have not started working on artisan commands, should I first finish issue#76. It is critical for marketing/legal purpose.

mhetreramesh commented 8 years ago

Yes that's true, most of it from the commands. I have already worked on it a bit, but considering the priority I think for now you can work on #76. After that I will take this up.

Thanks!

kapilsharma commented 8 years ago

Great, there won't be many changes. I'll not do any refactoring, just add twitter handle command so it must be easy merge for you, without many conflicts. Will let you know once it is updated on master branch.

kapilsharma commented 8 years ago

Ramesh, issue 76 done and committed on master. I just added few lines so there must not be any conflict. You can pull from upstream master and then merge master to your branch.

git checkout master
git remote add upstream https://github.com/phpreboot/website.git
git pull upstream master
git push origin master
git checkout <your-branch>
git merge master

There is a new migration file so please also run php artisan migrate locally.

Let me know in case of any doubt.

kapilsharma commented 8 years ago

Ramesh, I just had a quick look at your fork through https://github.com/phpreboot/website/network

It seems you are working on master branch. Thats fine for now but in git workflow, master always represent production, in our case, code live on phpreboot.com.

With commits on master, you will loose local version of current live site.( Necessary to do some testing as we can't debug on live)

Don't take otherwise but just to share workflow we should follow.

On main git repo (phpreboot/website) we have 2 branch.

So I recommend you to create development branch locally. Push all changes to development branch of your fork and send pull request from development branch of your fork to development branch of main repo.

kapilsharma commented 8 years ago

Ramesh, while working on this issue, you should have good understanding on commands. Please check if you can also pick issue #56 #57 & #58

mhetreramesh commented 8 years ago

Yes Kapil, I understand the work flow & I was under the impression that I am following the correct pattern. Now onwords I will make sure to follow the workflow strictly. Also I'll have a look at the mentioned issues.

mhetreramesh commented 8 years ago

Hi Kapil, I have fixed many coding standard issues from the code. Unfortunately I was not able to bring code quality score to any more up. I found there are lot of complexity in the Commands code and its kind of time consuming to fix those complexities. Here is the complex file link: https://www.dropbox.com/s/ukdtd549f8ir2y2/Screen%20Shot%202016-09-18%20at%209.40.10%20PM.png?dl=0

I would recommend to keep this task on low priority for now and we can focus on this again later on whenever we are going to write tests. What is your thought?

kapilsharma commented 8 years ago

No need to fix it 100%. Such tasks can't be done 100% in one go. Just commit whatever work is done. If needed, create new issue for remaining task.

No doubt there are many issues. It was crucial part of initial version so for me, priority was to get MVP ASAP. I know there are many issues.

Make it low priority, no problem. I just recommend to commit whatever already done.

mhetreramesh commented 8 years ago

Raised a pull request for issue fixes and created a task(#107) in the backlog for future code quality improvement.