These are chat archives for alariva/timegridDevelopment

27th
Nov 2016
draganrakovic
@draganrakovic
Nov 27 2016 12:52
Hi @alariva , I installed your project, and i would like to contribute, I saw this issue timegridio/timegrid#112 and it's my first time to trying to contribute to github project. However, I have some experience in development, but i want to learn more, so if you want some help I will be happy to do this one and lot more. :)
Ariel Vallese
@alariva
Nov 27 2016 14:24
Hi @draganrakovic
Thanks for showing up!
Let me know what can I help with
draganrakovic
@draganrakovic
Nov 27 2016 15:40
So, if I understand it correctly, for this issue I just need to add link to clients profile, and if the name is empty to display n/a (but i'm couldn't create client without name, the fields are required)?
Ariel Vallese
@alariva
Nov 27 2016 15:41
Right
You have an example of displaying client name, in the addressbook listing.. That is the contacts index
draganrakovic
@draganrakovic
Nov 27 2016 15:43
I allready did it, you can see it here http://prntscr.com/dcem52
Ariel Vallese
@alariva
Nov 27 2016 15:47
Looks good
There is a helper function
Do you make use of that?
Ill go now, and get back in about an hour or something
draganrakovic
@draganrakovic
Nov 27 2016 15:52
Yeah, i used str_links( route(link), fullname)
Ok, ttyl
Ariel Vallese
@alariva
Nov 27 2016 17:04
Imback
So it looks like you got familiar pretty well
draganrakovic
@draganrakovic
Nov 27 2016 17:10
I check it in the another view to see what did you use, i did it that way
So, as I said, i'm new to github, and I'm not sure what is my next step
Ariel Vallese
@alariva
Nov 27 2016 17:11
It looks like ready for PR
Oh.. i see
draganrakovic
@draganrakovic
Nov 27 2016 17:11
I forked you project
Ariel Vallese
@alariva
Nov 27 2016 17:11
Are you familiar with git?
Like pulling, pushing, committing
draganrakovic
@draganrakovic
Nov 27 2016 17:12
yes I'm,
Ariel Vallese
@alariva
Nov 27 2016 17:13
So
Basically
You need to add the change and commit it, then push to your fork repo. Best is to first branch out.
draganrakovic
@draganrakovic
Nov 27 2016 17:15
Should I do git remote add upstream your_project_url ?
I'm readding some tutorials and they said I should do that to stay on the same track as your project
Ariel Vallese
@alariva
Nov 27 2016 17:18
You are on track
Ill give you some reference articles
From that point you will see the step-by-step fork, branch, commit, push and PR
It's really easy, you can tacke that in minutes :)
draganrakovic
@draganrakovic
Nov 27 2016 17:31
Oh ok, sorry for asking it again :)
I need to go now, will be back in 2 hours, thanks for help :)
Ariel Vallese
@alariva
Nov 27 2016 17:31
Sure, no worries
Go ahead with that and let me know once you resume :)
draganrakovic
@draganrakovic
Nov 27 2016 17:32
will do :)
draganrakovic
@draganrakovic
Nov 27 2016 21:58
I think i got it, I sync branches and pushed it to new branc, and create pull request, when you see it let me know if everything is ok.
Ariel Vallese
@alariva
Nov 27 2016 22:01
I check it..
Yeah, looks just perfect
🚀
So, one mention here
This is a small change, we actually dont need more than this, but for the long run practice... Did you run tests locally?
draganrakovic
@draganrakovic
Nov 27 2016 22:06
No, I didn't, I don't have experience with tests
I just checked it manually
Ariel Vallese
@alariva
Nov 27 2016 22:08
Sure, so let me explain you so you can get things one step further.
With this particular case we dont need to run local tests for two reasons:
The change is so small an trivial that testing manually and reading the code we can tell that it's not breaking anything. Plus, tests are still run online... by the Travis-CI build.
get that?
draganrakovic
@draganrakovic
Nov 27 2016 22:11
Yeah
Ariel Vallese
@alariva
Nov 27 2016 22:11
draganrakovic
@draganrakovic
Nov 27 2016 22:13
so you are testing it now, if I understand
Ariel Vallese
@alariva
Nov 27 2016 22:13
right
Tests are automatically run for all PRs
but testing locally lets you save time by making sure that you dont break anything, and commit and push and create PR with confidence
draganrakovic
@draganrakovic
Nov 27 2016 22:19
Ok, i got it, so do I need to add something to my enviroment for testing it locally?
Ariel Vallese
@alariva
Nov 27 2016 22:19
what type of environment are you using?
draganrakovic
@draganrakovic
Nov 27 2016 22:20
I am now on Windows, so I use XAMPP
Ariel Vallese
@alariva
Nov 27 2016 22:20
So, check if you get it working with some of this info
Meanwhile.... merged!
draganrakovic
@draganrakovic
Nov 27 2016 22:25
Thanks, this was very small bug, but I hope that I'll help you more :)
Ariel Vallese
@alariva
Nov 27 2016 22:25
Sure, welcome anytime
Congrats for your first PR
:sparkles:
Screenshot from 2016-11-27 19:28:33.png
And.. this is your boy in action... already on demo
draganrakovic
@draganrakovic
Nov 27 2016 22:30
So far so good :D
Ariel Vallese
@alariva
Nov 27 2016 22:30
:clap:
draganrakovic
@draganrakovic
Nov 27 2016 22:45
I set up db, and run vendor/bin/phpunit and it's green
Is that all, or should I do that other part 'Generating your own local Test Coverage report'
Ariel Vallese
@alariva
Nov 27 2016 22:47
no, that's just perfect
you did it! :)
draganrakovic
@draganrakovic
Nov 27 2016 22:47
And now i should delete my branch? :)
Ariel Vallese
@alariva
Nov 27 2016 22:47
You can delete your branch, yeah
its already merged
git branch -d issue/blahlbah
The code coverage is automatically generated here
draganrakovic
@draganrakovic
Nov 27 2016 22:49
I saw that link, but I'm not sure what to look exactly :)
Ariel Vallese
@alariva
Nov 27 2016 22:51
Kinda advanced for now...
but basically, it is a map of the production code lines that are covered by tests
this means that the green (covered) lines are tested, and those in red are not. Meaning that if you change something in red, the tests will not notice that change.
draganrakovic
@draganrakovic
Nov 27 2016 22:53
Sorry, I know that I have so many questions, you don't need to answer to all :)
Ariel Vallese
@alariva
Nov 27 2016 22:54
its good if you have questions
thats the best
i will tell as much as i can go... or provide you with some useful article
draganrakovic
@draganrakovic
Nov 27 2016 22:57
Than, guess what...I have more :)
I'm useing PHPStorm, and it creates me .idea/, and ofc you don't have it in project, so would it be possible to put it in gitignore file?
Ariel Vallese
@alariva
Nov 27 2016 22:58
right
i really dont know how should this be handled in the best practice
but i guess its harmless to add it to the gitignore
if you want to send the pr for that, will be perfect
draganrakovic
@draganrakovic
Nov 27 2016 23:00
That's my guess too, I googled it and the all say just add it to gitignore
Ariel Vallese
@alariva
Nov 27 2016 23:00
lets see
draganrakovic
@draganrakovic
Nov 27 2016 23:01
I can't delete my branch, it's saying me 'it's not fully merged'
I think it's becouse that folder
Ariel Vallese
@alariva
Nov 27 2016 23:02
probably
then do this
git stash
git checkout master
git branch -D issue/blahblah
git stash pop
draganrakovic
@draganrakovic
Nov 27 2016 23:06
when I did git stash it said 'No local changes to save'
But I was able to deleted branch with capital -D
Ariel Vallese
@alariva
Nov 27 2016 23:07
cool
draganrakovic
@draganrakovic
Nov 27 2016 23:09
should i create new branch 'issue/gitignore' and update gitignore?
Ariel Vallese
@alariva
Nov 27 2016 23:09
uhm...
not reeaally
but it would be a good practice
btw you can name it tweak/add-idefiles-to-gitignore
dont be afraid of bein descriptive
and keep issue/ to tackle standing github issues
but this is just a convention
draganrakovic
@draganrakovic
Nov 27 2016 23:11
oh ok
Ariel Vallese
@alariva
Nov 27 2016 23:12
the thing with branching out
is that you reduce a lot the chances of generating merge conflicts
and even when they happen its easier to handle the cases
However, I found this
(about the gitignore)
Ariel Vallese
@alariva
Nov 27 2016 23:32
@draganrakovic , I'll be out for biking now
btw... whats your timezone?
draganrakovic
@draganrakovic
Nov 27 2016 23:35
Ok, have fun.
I'm from Serbia, so it's +1
Ariel Vallese
@alariva
Nov 27 2016 23:35
cool
I have a friend from Belgrade
Ill go visit one day :)
draganrakovic
@draganrakovic
Nov 27 2016 23:36
Oh cool, Belgrade is fun, you should come :)
Ariel Vallese
@alariva
Nov 27 2016 23:37
serbian and argentina cultures have their common sides :):):)
draganrakovic
@draganrakovic
Nov 27 2016 23:38
I know, that's why I think you will like it here :)
Ariel Vallese
@alariva
Nov 27 2016 23:39
so I will take that in my todo list and hope for ракија
draganrakovic
@draganrakovic
Nov 27 2016 23:40
haha, rakija is a MUST on that list :D
Ariel Vallese
@alariva
Nov 27 2016 23:41
number one!
ill take it
so now on my :bike: ... catch up whenever, just let me know!
thanks lot for contributions!!
draganrakovic
@draganrakovic
Nov 27 2016 23:44
np man, when I'm free I will see how can I help more
Ariel Vallese
@alariva
Nov 27 2016 23:46
;) :octocat: :ok_hand: