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

06-23-2018 , 03:49 PM
We have a rule that you need a minimum of 3 approvals from any dev OR one approval from one of the senior devs. This gives the junior devs a chance to participate which I think they should. Only the QA can merge.

So the process is like this:
1. Dev assigns himself to a task and puts it "in progress"
2. Dev completes dev work, commits, creates a PR, places task "in review"
3. Other devs review task.
4. Once task has enough approvals, original dev places task in "ready for QA"
5. QA starts qa work on task, places task "in QA"
6. If task passes, qa merges task to master

During the review process, if comments pop up then the original dev can make adjustments to the code and commit them. That automatically refreshes the PR with the new diff and removes any approvals. It's also automatically put back into "in review".

Last edited by Wolfram; 06-23-2018 at 03:57 PM.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-23-2018 , 04:54 PM
Quote:
Originally Posted by jmakin
yea me too - JJ, when you merge to master it goes to release right away, or fairly quickly??
For us, all our services go through
* build/test
* deploy to CI
* run end-to-end tests on CI
* deploy to prod
* run end-to-end tests on prod

Our monolith is the same except that someone has to press a button to start the deploy to prod.

You can put in a "break" between CI and prod for the services, but you only do that if you expect that there could be a problem or you want to time the release.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-23-2018 , 08:33 PM
Quote:
Originally Posted by Larry Legend
Totally wrong u less you mean junior to be like less than 3-6 months at the company. After 6 months on a team everyone should be approving code, ideally on just a rotational basis.


I think even less than this. I look at it like releasing to production. Ideally people should be doing it within days of joining. And it’s up to the more senior people on the team to make sure it goes successfully by giving tasks that are appropriate for the persons skill/experience level.

Obviously this depends a lot on the type of work you’re doing and the processes your company has. But if you’re doing small and frequent releases there are almost always changes that are low enough risk that everybody can give a reasonable second opinion.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-23-2018 , 09:34 PM
Quote:
Originally Posted by jjshabado
Why not? Honestly, there are a large number of non critical code reviews that young developers are perfectly fine reviewing.
Quote:
Originally Posted by Larry Legend
Totally wrong u less you mean junior to be like less than 3-6 months at the company. After 6 months on a team everyone should be approving code, ideally on just a rotational basis.
I guess I just suck at coding then (not surprising) bc if I was a reviewer then I would miss tons of stuff.

For code to get reviewed and allowed into production I think there should be fairly stringent policies and that includes only the best being allowed to give approval.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-23-2018 , 09:36 PM
like, it is utterly baffling to me that you guys advocate giving anyone but the most qualified the ability to approve code for production.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-23-2018 , 09:38 PM
Review != giving approval
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-23-2018 , 09:42 PM
Quote:
Originally Posted by Larry Legend
One of the things I love about my job is that I was very quickly responsible to review the code of others who who much more experienced than me.

For anything beyond trivial my process is usually this:
Make sure I have a branch that is identical to what they are merging into.
Make a new branch locally and pull their PR into it and then merge into their destination.
Build in one terminal (sometimes with breakpoints always with tests and linting)
Open the project in another terminal
Have the PR diff open in github
Review the code, quickly test the end user experience that it is supposed to be solving/fixing. Try and consider any gotchas in the UI that could cause a weird state.

Click accept, click merge
larry, I like your process bc it seems to me that you recognize the most important aspects. but most of this could be automated. not by you ofc. but it is absurd that a code reviewer should need to build the code and run tests. that is up to the developer and there are many tools that automate such that the PR cant even be submitted until it passes.

seems totally absurd that a PR would not have run unit tests and a build.

also, likewise, it is absurd that the code reviewr would need to test business functionality. that is on the developer and should certainly be reviewed by business and the QA.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-23-2018 , 09:44 PM
Quote:
Originally Posted by jmakin
Review != giving approval
jmakin, I clarified that and you can see there is still a lot of pushback against my position.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-23-2018 , 09:59 PM
I mean giving approval.

You learn by doing and reviewing code is a skill that can be developed. Being told it’s on you to review and approve code also breeds a culture of developer responsibility and trust which I think is really important.

I think you guys might also over emphasize the magnitude of difference between a junior developers review and an experienced developer. Hell, ITT we’ve had two people say they don’t even really try to understand the code and instead are looking for more superficial things.

There’s also a certain amount of cultural context here. If you’re in a fast moving company with solid test/build/release/monitoring processes and tools it’s rarely a big deal to release a bug (and if you’re working on something where it would be a big deal - obviously you require more stringent reviews and testing). It doesn’t make sense to have every line of code block on a senior person and it probably doesn’t make sense to have them do all that work too.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-23-2018 , 11:01 PM
We use BB pipeline to build/run tests after every commit.

I view a PR in github desktop. If there are complicated UI changes I will pull the branch and run locally.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-24-2018 , 09:26 AM
Testing the functionality of the code isn't something I do 100% of the time, but it definitely happens more than 50% of the time.

We have dedicated QA but I will regularly find obscure bugs that they dont.

A new guy on my team struggled with a piece of functionality for about 2 weeks, when he finished I tested it and found a really obscure bug that was recoverable. It is unlikely to be triggered much for the first several months or year in prod. QA passed it. I showed him the bug but told him to not worry about it for now (because we have other priorities than a recoverable obscure bug) but it is still a bug that I found instantly and QA never did.

When I read someone's code, I can immediately see things that may be totally fine, or might be a place where a bug lies, testing the end experience is a great way to determine which it is.

It also allows me to become very knowledgeable about our products, we recently released a new app that is fairly simple (30 components, 10 containers) and i have 90%+ of it in my memory and know every feature.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-24-2018 , 09:57 AM
lot of good points. thanks guys.

Quote:
Originally Posted by jjshabado
I mean giving approval.

You learn by doing and reviewing code is a skill that can be developed. Being told it’s on you to review and approve code also breeds a culture of developer responsibility and trust which I think is really important.

I think you guys might also over emphasize the magnitude of difference between a junior developers review and an experienced developer. Hell, ITT we’ve had two people say they don’t even really try to understand the code and instead are looking for more superficial things.

There’s also a certain amount of cultural context here. If you’re in a fast moving company with solid test/build/release/monitoring processes and tools it’s rarely a big deal to release a bug (and if you’re working on something where it would be a big deal - obviously you require more stringent reviews and testing). It doesn’t make sense to have every line of code block on a senior person and it probably doesn’t make sense to have them do all that work too.
my company is quite large and we are absolutely terrified of bugs. like, we have tons of testing and it takes a long time to put out new releases. at least a month from checkin to production.

your points about culture and responsibility (and as a corollary developer learning) makes sense. while there isnt much difference between myself (more junior) and most of the seniors and leads, our code reviewers are certainly a step above.

but we have had issues with PR sitting for awhile so maybe it would be a good idea to groom some more devs to become competent at PRs so the project is confident in them.

I still think there needs to be a fairly high level of understanding to have approval authority unless there is a way to sort PR by complexity and importance. And perhaps such a system should be implemented.

Quote:
Originally Posted by Larry Legend
Testing the functionality of the code isn't something I do 100% of the time, but it definitely happens more than 50% of the time.

We have dedicated QA but I will regularly find obscure bugs that they dont.

A new guy on my team struggled with a piece of functionality for about 2 weeks, when he finished I tested it and found a really obscure bug that was recoverable. It is unlikely to be triggered much for the first several months or year in prod. QA passed it. I showed him the bug but told him to not worry about it for now (because we have other priorities than a recoverable obscure bug) but it is still a bug that I found instantly and QA never did.

When I read someone's code, I can immediately see things that may be totally fine, or might be a place where a bug lies, testing the end experience is a great way to determine which it is.

It also allows me to become very knowledgeable about our products, we recently released a new app that is fairly simple (30 components, 10 containers) and i have 90%+ of it in my memory and know every feature.
this makes sense. I think I may start doing this on my own to develop my own knowledge.

still dont think you should be running tests. I mean, they should all pass or its not a PR.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-24-2018 , 10:41 AM
Quote:
Originally Posted by Victor
I still think there needs to be a fairly high level of understanding to have approval authority unless there is a way to sort PR by complexity and importance.

One small adjustment is that you say all junior devs need to have a senior dev approve their reviews but you encourage the senior devs to use their experience to know which PRs are high risk and which are low risk and request more junior people to grant approvals where appropriate.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-24-2018 , 10:51 AM
The problem with testing code functionality at our giant company was that just to set up a test user - we'd have to interact with a dozen back-end systems run by different departments which we have no control over. Basically if the entire company had built everything to be testable from scratch we'd be fine. But some of these systems date back to the late 80s I think.

The amount of work involved in setting up test users, with specific conditions, etc. was such an expert-level, time-consuming task that it made sense to have a separate SQA department to focus on it. If engineers had to test all our own functionality to the level SQA did, we'd spend over half our hours just doing crap to set up the test - which is harder if you're not the dev actively working on that feature.

Anonymous user behavior was still often a challenge because the test involved a user in a specific market segment, or specific rare type of scheduled movie or TV show.

I'm not saying this system is better, it's probably worse. But it is kinda freeing as a lead developer to look at some junior dev's code and say "Ok, this code looks sound. As long as it works I'm ok with it."

Last edited by suzzer99; 06-24-2018 at 10:56 AM.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-24-2018 , 01:52 PM
Yea that's a bit of an issue here too.

I always make sure to keep our backends up to date and I run a tail -F on the main log files in two terminal windows on the top of my right side vertical monitor to just have constant logging and make sure everything is good and watch the applications. It takes up minimal screen space and it's kinda like reading the matrix and multiple backend devs have remarked that it looks cool as hell and it also has the result of making me fluent in apache/java logs.

It can take somewhat significant sets of configs to get environments to mimic what us being done/fixed, but as long as you are good with the systems and have then running on your local then it's really only minutes to set everything up.

We were fixing some finals bugs before a fairly large release recently and a backend dev and I realized one of my teammates hadn't build the backend successfully in several months and was just working on a limited fake api we had build that was no longer necessary (for a time when we were well ahead of the backend) it was a wtf moment for both of us, and further solidified my confidence in my approach.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-24-2018 , 01:54 PM
Yea on the topic of expert level setup of accounts, I feel like knowing that stuff is so incredibly valuable. So often there is code that people think will work but testing it is nearly impossible. The know-how to set it up quickly makes fixing bugs soooooooo much faster.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-24-2018 , 08:06 PM
We couldn’t run the user creation systems locally. We had to create test users into their staging environment using their APIs. Whole different department that barely speaks to us.

It’s probably a common scenario at companies that had big complex user management systems in place before the new internet even came along.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-24-2018 , 09:31 PM
As far as job boards go, if you were applying to jobs which ones would you check and which ones would you avoid entirely?

Spoiler:
If anyone is hiring for a web dev position in the Seattle area shoot me a pm
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-26-2018 , 12:23 AM
does anyone here prefer Angular to Vue? if so, why?
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-26-2018 , 01:21 AM
Quote:
Originally Posted by Craggoo
As far as job boards go, if you were applying to jobs which ones would you check and which ones would you avoid entirely?

Spoiler:
If anyone is hiring for a web dev position in the Seattle area shoot me a pm
I would draft a list of companies on a spreadsheet that are in my area.

Then jump onto LinkedIn and see if I have any connections on there.

Then simultaneously reach out to all of them.

Do the interview.

Leverage offers.

???

PROFIT!
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-26-2018 , 11:55 AM
Quote:
Originally Posted by Barrin6
I would draft a list of companies on a spreadsheet that are in my area.

Then jump onto LinkedIn and see if I have any connections on there.

Then simultaneously reach out to all of them.

Do the interview.

Leverage offers.

???

PROFIT!
I'm not exactly in a techy area right now. I'm planning to move with this next job (hopefully Seattle area). So that approach doesn't really work.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-26-2018 , 01:29 PM
They just announced several site closures at my old (well current for 1 more week) company. Kinda confirms to myself my decision to leave.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-26-2018 , 05:17 PM
Quote:
Originally Posted by Barrin6
I would draft a list of companies on a spreadsheet that are in my area.

Then jump onto LinkedIn and see if I have any connections on there.

Then simultaneously reach out to all of them.

Do the interview.

Leverage offers.

???

PROFIT!
This is basically what I need to do. But the thought of it gives me a headache and makes me want to curl up in a ball and take a nap. I'm a programmer not a professional interviewee dammit. I'll do it of course because I have to.

Unless ... someone should offer this service for a fee. Like a recruiter but somehow we need to fix the incentives so they don't just go bananas to push you into the first offer. The big incentive problem with recruiters is they get no repeat business from job-seekers. So it doesn't matter how much they piss you off, as long as they close the deal.

So I think the only way it can work is by reputation. Basically offer the service for flat fee, with some kind of bonus for # of interviews and a final modest bonus when one is accepted. The key is that you live by your online ratings. If you piss too many people off your business suffers. If you get glowing reviews people beat a path to your door.

Tour guides at some places work on a similar model - where they show you their big book of hand-written reviews from other tourists who used them. Unless they steal someone else's book of course. But that would be harder to do online.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-26-2018 , 05:52 PM
I've spent countless hours thinking about this/ talking to experts/etc. and I ultimately think the problem is still too difficult to solve, but it is getting easier.

I think the current generation of recruiting like services need to continue to mature and be accepted until the real solution can come into place.

Ultimately, I think some sort of standard application that potentially includes a video interview, etc. that someone can watch along with resume/paperwork/certification, something to standardize it so that you can do 1 process and simultaneously apply to virtually infinite companies.

It is crazy to be forced to take multiple days out of work, ruining your reputation or your paid time off, with multiple companies. It could easily require 2 weeks total of time off in order to interview with 4-5 companies until offer stage.
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote
06-26-2018 , 08:22 PM
Has anyone worked with websockets at all?

I want to consume a service which is running a MigratoryData server. MigratoryData are a company with a proprietary implementation of websockets. They have client APIs but getting hold of them is proving difficult - you have to register to download stuff from their website, I tried but they won't approve the registration. I could email them but I'm not a paying customer so I doubt they care. What I'm doing is a bit shady in that I intend to consume a service which is not really intended for use as an API (it's a scores service which is used by a website to push live score data to client browsers).

Will I probably be able to consume the service with a standard websockets client?
** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** Quote

      
m