Open Side Menu Go to the Top

01-09-2019 , 05:52 PM
Has anyone tried to do logging with AWS lambda? I'd like to just use the built in cloudwatch logs as much as possible. But I'd also like good stuff like log levels, and automatically tacking on stuff like requestID, sessionID, timer info, etc. - w/o each call to a log having to worry or be aware of it. We used bunyan for node logging in the past but I'm not sure that applies here.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD **
$25m Guaranteed WPM on CoinPoker
Join the action now
Daily Rewards • Splash Pots • CoinRaces
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD **
01-09-2019 , 07:26 PM
I must have been extremely lucky in that almost everyone I've worked with has been super nice and easy to work with. Not all of them were great engineers, but they were at least easy to work with.

In regards to code review process, we use the Git / Gerrit / Jenkins stack for reviewing, smoke testing, and pushing code. Need to get a +2 at a minimum, but prefer also having a second +1 for all but super trivial changes. Also need to get a pass from Jenkins.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-09-2019 , 07:42 PM
Yeah, having a process that simply doesn't allow merges with the proper code review pretty much forces people to get their process in line.

Also, you do not have to be an expert on the code base in question to give a code review. It definitely helps, and it might take longer without it, but it's not a requirement.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-09-2019 , 07:44 PM
So in that scenario - how often is it just a 30 second glance and a rubber stamp?
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-09-2019 , 08:24 PM
It probably depends on how much accountability you have built into your process.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-09-2019 , 10:44 PM
Quote:
Originally Posted by suzzer99
So in that scenario - how often is it just a 30 second glance and a rubber stamp?
It's going to depend on the person. I look for patterns that I think lead to better outcomes.

Were tests written? Do I understand them? This should be possible without domain knowledge and probably without even deep knowledge of the language.

Provided you know the language you can look for anti-patterns, you can see if the code has good readability. You look for repetitions and giant functions and deeply nested ifs, all that stuff that you know is bad, regardless of what the code is supposed to do. You look for stuff that has a high probability of causing errors, sloppy stuff like
if (x = 0) {}
if your language allows for it, etc.

You do your best to understand what the change is trying to do, you ask some questions.

It's not uncommon for code reviews by a non-expert in the area to be pretty short. A lot of it also comes down to trust between you and the other person.

It helps if there are other systems in place - automated testing, static analysis, and so forth.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-09-2019 , 10:46 PM
LOL tests written, I wish I lived in the world you guys do.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-09-2019 , 11:39 PM
uh is it really that hard to say "hey if you put in a pr and the code coverage drops your pr won't be accepted?"
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:14 AM
The majority of workplaces don't do testing. At all.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:20 AM
Quote:
Originally Posted by Grue
uh is it really that hard to say "hey if you put in a pr and the code coverage drops your pr won't be accepted?"


I dont think code coverage is a good metric of code quality at all

But lol code coverage. Yea we totally measure that
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:28 AM
We tried - devs wrote a bunch of useless tests to get to the magic threshold and missed important tests. They were massively under the gun, trying to navigate a whole new microservices platform, and the app wasn't being really governed for consistency at all. IE - code coverage was like 13th on the list of problems.

I'm sure it works at places that are well run. But then again I imagine those are the places that need two approvers for every PR the least.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:31 AM
Quote:
Originally Posted by RustyBrooks
It's going to depend on the person. I look for patterns that I think lead to better outcomes.

Were tests written? Do I understand them? This should be possible without domain knowledge and probably without even deep knowledge of the language.

Provided you know the language you can look for anti-patterns, you can see if the code has good readability. You look for repetitions and giant functions and deeply nested ifs, all that stuff that you know is bad, regardless of what the code is supposed to do. You look for stuff that has a high probability of causing errors, sloppy stuff like
if (x = 0) {}
if your language allows for it, etc.

You do your best to understand what the change is trying to do, you ask some questions.

It's not uncommon for code reviews by a non-expert in the area to be pretty short. A lot of it also comes down to trust between you and the other person.

It helps if there are other systems in place - automated testing, static analysis, and so forth.
So you're describing stuff a linter and static analysis should catch, which a lead would never do (or your whole shop is in trouble) and then a rubber stamp by a non-expert.

Which is my point - just drop the rubber stamp part in these cases - especially if it's backing up PRs for a week or something.

I wonder if some of this problem is from having a basically flat structure and not a clear tier of leads vs non-leads. So everyone knows the times you really need to do a review and the times you just skim through it. But you don't want to just come out and allow no reviews - because then you're singling out the devs who really need reviews.

Just guessing as I've always worked in an environment with a clear lead tier.

I'm just saying I think PRs don't sit for more than a day, 2 days max - should be the most sacred rule. I think there's way more potential for issues, and it's frustrating for the devs, etc. Obviously some things are super independent and can sit longer. I'm thinking about the tightly integrated stuff with a team collaborating.

Last edited by suzzer99; 01-10-2019 at 12:37 AM.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:48 AM
A good linter can catch a lot of stuff. But I've caught tons of stuff in code reviews that no linter would ever get, even without knowing the code base or even sometimes the language. I interviewed someplace where as part of the (all day) interview, I had to perform a code review on someone's PR. It was Java, which I barely know, and I found some logical errors. There were probably some java-specific errors I did not catch.

As to "most places don't test" I guess I don't really know. I do have to say though, that I have never even *interviewed* someplace that didn't stress testing as a core approach. And I haven't worked anywhere that didn't do testing in 20 years, except for my first job, which was a literal seat of the pants startup. That was my first job out of college and I was the sole developer for while, and then 2 of us for a fairly long while. I don't know if (automated) testing was a common methodology at that point or not but I was basically unaware of it as a concept aside from like writing some simple scripts that you'd use to see if stuff basically worked.

But there's a great deal of self-selection going on. It's quite likely I don't choose to interview at places that don't have good practices, and/or they don't choose to interview me.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:50 AM
And yes, PRs should be accepted or rejected as soon as possible. A day or 2 seems very reasonable, and that should be worst case. Most of them should be accepted same day, within hours or minutes if possible.

Personally I really prefer fixing and closing something to having 5 things open that are in various stages of acceptance and deployment. For most of my last few jobs, it was typical for things to get fixed, tested, PRed and deployed same day, usually several times/day.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:51 AM
At my last company I probably deleted more tests than i contributed. Poor/pointless tests really annoy me.

We had a project I didnt contribute that much to but reviewed all the PRs for and it had tons of tests that checked hardcoded text that the marketing team wanted to change constantly. This dev stopped caring about any failing tests (and was lazy so important ones would sometimes slip past him) so I put an end to those tests.

Good test coverage is a nice feeling but I prefer no tests to bad ones for sure.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:53 AM
Quote:
Originally Posted by jmakin
I dont think code coverage is a good metric of code quality at all
Didn't say that either yo. But its.. a metric. Something you can push back against.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:56 AM
Quote:
Originally Posted by RustyBrooks
As to "most places don't test" I guess I don't really know. I do have to say though, that I have never even *interviewed* someplace that didn't stress testing as a core approach.
Me either. I've also never interviewed at a place that didn't claim to have a good work environment and excellent promotion opportunities.

Edit: I'm talking about automated testing here obviously. The thing is, "testing" is a word with a lot of wiggle room. Unless places say like "we use xUnit and our test coverage is x% as measured by y" then in my experience their testing is frequently nonexistent.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 01:12 AM
We did enough testing that the bosses could put it on their powerpoints. That's all they cared about - so that's all that got done.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 12:41 PM
Quote:
Originally Posted by ChrisV
Me either. I've also never interviewed at a place that didn't claim to have a good work environment and excellent promotion opportunities.
Sure, but I'm talking about getting grilled about how much I know about testing and what my approach is and what I think about TDD and how I'd test this or that, etc.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 02:51 PM
I think TDD is awful, am I an idiot?
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 03:07 PM
I will say one thing - that flowchart I posted - I didn't do TDD, but I couldn't wait to write the unit tests for that. Because only then would I be confident that it actually works and I have a handle on everything that's going on. I'd feel naked trying to put that into service w/o good unit tests (which I'm now working on incorporating into integration tests as well).

That has to be some kind of inflection point.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 03:19 PM
Well I should amend that, for building endpoints I could do TDD and I would find it decent.

So I guess in some cases it can be useful but I feel like building an entire application using it is horrible.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 03:21 PM
Are we talking academic TDD where we write tests first? Or just TDD as testing in general? I’m all for extensive testing but am not a fan of writing the tests first.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 04:03 PM
TDD stands for test driven development - which afaik means you're supposed to write the tests first. Curious if anyone actually does this.

The one that makes more sense to me is BDD - which we have actually written first (cucumber tests at the side job startup) - since the tests basically read like requirements.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
01-10-2019 , 04:14 PM
Quote:
Originally Posted by Larry Legend
Well I should amend that, for building endpoints I could do TDD and I would find it decent.

So I guess in some cases it can be useful but I feel like building an entire application using it is horrible.
I think the answer is always "it depends". People who get caught up in any paradigm or technology are going to over-apply it.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD **
$25m Guaranteed WPM on CoinPoker
Join the action now
Daily Rewards • Splash Pots • CoinRaces
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD **

      
m