Open Side Menu Go to the Top
Register
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** ** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD **

12-08-2017 , 12:11 AM
If you really cared you'd stage and commit each line by itself, explaining what the change was.

Joking aside, breaking it up by file makes no sense. If you are changing 2 files to fix a bug, then the atomic unit of the fix is both files. You're actually breaking the efficacy of tools like git bisect because it could check out a state in between your 2 commits, and it's likely that wouldn't be a consistent working state.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 02:00 AM
I would love to see the code base that only requires one or two files to change for a fix.

For me alone, it isn't a real issue and I just add all, but working with others, all fixes go into a single push. Eh, it's probably not optimal, but I don't always know how to indicate each change and why it would be important.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 02:03 AM
At the bootcamp I went to they tried to get us to save each file individually, I didn't get it.

Am I doing something wrong with:

git diff
git add .
git commit
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 04:42 AM
I’m surprised you guys commit everything so often through the command line.

I like reviewing the diff as a final code review (which is definitely better in a GUI) before pushing my changes out for other people to review. And I like being able to easily separate different changes (say I change logic as well as fix formatting). The unit of change definitely isn’t a file but I do break my work up into separate commits fairly often.

Although things like git bisect are pretty useless for a lot of the code I work with. So clarity of commit is definitely better than working state after each commit. We use PRs to be the larger organization of work.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 08:28 AM
you guys that commit stuff often and break up your commits by files or whatever, do you guys squash your commits then? doesnt that kina defeat the purpose?

Quote:
Originally Posted by RustyBrooks
If you really cared you'd stage and commit each line by itself, explaining what the change was.

Joking aside, breaking it up by file makes no sense. If you are changing 2 files to fix a bug, then the atomic unit of the fix is both files. You're actually breaking the efficacy of tools like git bisect because it could check out a state in between your 2 commits, and it's likely that wouldn't be a consistent working state.
my understanding is that squashing commits keeps this from being an issue.

Last edited by Victor; 12-08-2017 at 08:41 AM.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 11:13 AM
Quote:
Originally Posted by Victor
you guys that commit stuff often and break up your commits by files or whatever, do you guys squash your commits then? doesnt that kina defeat the purpose?



my understanding is that squashing commits keeps this from being an issue.
Ha, we squash at my current company, so youre right its basically a total waste, but just habit at this point.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 01:23 PM
Uh anyone not squash? What a nightmare.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 01:35 PM
We don't squash anything
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 02:03 PM
You don't no ff merge to master?
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 02:58 PM
Quote:
Originally Posted by Grue
You don't no ff merge to master?
I'm not 100% sure what you're asking. Everything goes to master. If you're working in a branch, when you're done, you merge the whole thing to master - without rebasing or any other means of squashing commits.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 04:56 PM
my understanding is that squashing commits is not necessary but it makes fixing merge conflicts a ton easier. otherwise there may be a situation where you need to fix the merge conflicts for every commit you made, rather than just the final version of your code.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 05:07 PM
Quote:
Originally Posted by Victor
my understanding is that squashing commits is not necessary but it makes fixing merge conflicts a ton easier. otherwise there may be a situation where you need to fix the merge conflicts for every commit you made, rather than just the final version of your code.
This is only true if you fast-forward - if you merged at the end, you can always revert that single merge commit.

I'm personally not a big fan of squashing but looking at git history linearly for a dev/master branch without squashing can be confusing because things are generally ordered by when the commit was made, not when the commit was introduced to the branch.

Edit: I misread what you said - what I'm saying has to do with reverting, not fixing merge conflicts. I'm not aware of any situation where merge conflicts have to be fixed for each commit - the merge itself is the commit that introduced conflicts that need to be resolved.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 06:01 PM
So I asked before about doing boolean checks in render for props in react, and grue said it's how everyone does it and mentioned using {boolean && thing} in your JSX.

I've never liked this, I would rather have components have as little logic as possible.

A design we spoke about today was using a container component to send mapStateToProps and mapDispatchToProps to connect, and then passing the returned function your child dumb component.

So in the container you do

export default connect (
mapStateToProps,
mapDispatchToProps
)(ChildComponent)

This gives all the props and dispatched actions to the child component through connect and allows you to remove all that junk from your child's render method.

Also, you don't need to declare the container as a react component because connect will return you a function that then wraps the argument you pass in as a react component. So you can create containers that house all the destructuring and will have a subscription to the store to re-render the child whenever any of those props that you destructured changes. It seems like a way more elegant way to use props in a child component and not crowd your render method with a bunch of props checking, allowing you to have a more pure child component that really just returns JSX with minimal else.

Obviously if you need to access lifecycle methods in the container or do other things than this won't be the way you do it, but for simple features consisting of not much more than crud actions, I feel like it's a superior design.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 06:07 PM
This is also the design pattern used in Dan Abramov's redux example where he does:

const FilterLink = connect(
mapStateToProps,
mapDispatchToProps
)(Link)

export default FilterLink
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 06:39 PM
Quote:
Originally Posted by Victor
my understanding is that squashing commits is not necessary but it makes fixing merge conflicts a ton easier. otherwise there may be a situation where you need to fix the merge conflicts for every commit you made, rather than just the final version of your code.
In the team I'm on merge conflicts are vanishingly rare. I've worked in bigger teams where it was somewhat more common but we're still talking like once a month or something.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 06:48 PM
how big was the bigger team? my project proly has 50 devs checking in routinely. they happen all the time. its not really a big deal, we cant merge if theres a conflict. its up to the dev to figure it out and make it right.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 07:11 PM
I still don't understand - what do merge conflicts have to do with squashing?
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 07:12 PM
Quote:
Originally Posted by Victor
how big was the bigger team? my project proly has 50 devs checking in routinely. they happen all the time. its not really a big deal, we cant merge if theres a conflict. its up to the dev to figure it out and make it right.
The largest team I've been on was probably 20-25 people. It probably just was not that common for 2 people to be working on the same thing. Again I wonder if this is more of a problem for FE people (same with the surprise at commits being just a few files). I can see that with angular or react you need to touch 9 files every time you change anything, and also there are going to be lots of files that most changes will need to modify.

That larger team was probably 70% C++ and the remaining bit was bash/python/perl/whatever scripts, plus the build system.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 07:39 PM
LL I didn't mean check for the existence of props in your render. If you're not using a prop in your component then don't pass it obv. The component/container pattern is fine and I use it when it works but its also a lot of extra code and 2 files vs 1.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 07:57 PM
Yea I wasn't clear on that.

I do feel like when I look at code snippets online I see a bunch of junk in render that can go into mapStateToProps tho.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 09:13 PM
Quote:
Originally Posted by RustyBrooks
The largest team I've been on was probably 20-25 people. It probably just was not that common for 2 people to be working on the same thing. Again I wonder if this is more of a problem for FE people (same with the surprise at commits being just a few files). I can see that with angular or react you need to touch 9 files every time you change anything, and also there are going to be lots of files that most changes will need to modify.

That larger team was probably 70% C++ and the remaining bit was bash/python/perl/whatever scripts, plus the build system.

I think it also comes down to how well the project is managed, along with how other devs are. In my last contract, I had to manually fix merge conflicts at least twice a day, and those fixes always introduced more bugs.

There was 4 of us. I was the only one using branches. Everyone else was pushing directly to master.

I'm shocked anyone uses fast forward while working on a team.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 10:39 PM
Quote:
Originally Posted by Victor
you guys that commit stuff often and break up your commits by files or whatever, do you guys squash your commits then? doesnt that kina defeat the purpose?



my understanding is that squashing commits keeps this from being an issue.


I only squash if I have messy commits or a lot of commits.

But even if we end up squashing separate commits can make code reviews easier even if the separate commits aren’t long lived.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-08-2017 , 10:51 PM
We do a pull request on a different branch. We can basically commit whatever the hell we want because when it's approved and merged it's squashed so we get one clean commit per bug/small feature in develop. Works very well
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-09-2017 , 04:53 AM
I've stubbornly refused to use the ternary operator for years because of a C++ professor that would randomly throw a question about it in every single exam and I'd mess up the syntax every time.

i used it in my last project and holy **** it's actually really useful. I'm such a scrub, lol.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
12-09-2017 , 05:20 AM
Go not having a ternary is a little annoying
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote

      
m