DEV Community: Andrey Bodoev

Do you think consultants has to be treated differently?

Andrey Bodoev — Sun, 17 May 2020 05:50:01 +0000

If you ever been in consultanting business, you may hear spooky stories, like so,

We have separate pantries for stuff and vendors. If you want to have access please chip in.

Question to community, do you think vendors or consultants has to be treated differently? Share your story.

Run remote retrospectives using Trello

Andrey Bodoev — Thu, 07 May 2020 11:39:05 +0000

Here's quick walkthrough how we run retrospectives using Trello boards. It's fairly simple workflow, and can be adapted with easy.

Few things to add,

this approach turns out works well for both remote and in office environments;
can be highly customized depends on needs;
I haven't say that in video, but it's removes frictions with post-its, and use cards instead, and creates a possibilities to have issues to discuss in advance, prior to retrospective.

I'm using Playbook framework by Atlassian as an example for this board

If you have any questions, please let me know in comments below.

Hidden power of Commit Guidelines

Andrey Bodoev — Sun, 03 May 2020 10:40:47 +0000

In our projects we adapted Commit Guidelines, with more or less standard variation of Angular Commit Guidelines. It deliver what it promise,

This leads to more readable messages that are easy to follow when looking through the project history. But also, we use the git commit messages to generate the change log.

Since we adapt such guidelines, I have discovered one powerful effect on a developer growth.

It's how you start thinking about code changes you commit to code base. Following questions starts to bubbling inside your head,

Does this change belong to this commit?
What's clear intent of your changes in?
What's reasoning and thoughts I can put in message?

And etc.

What happens now is that each commit is representing some type of change, with describing clear intent encapsulated inside of commit message.

Suddenly, you start reading git log (yes, for real), and if you need to do comparison between log histories you can do this by simply looking at the titles of commit messages.

Here's one example,

$ git log --pretty="%n    %s" --name-only

    test: has Cancel button, to check both confirm branches

cypress/integration/FunctionalTesting_Suite/CompanyProfilePage/company_profile_page.spec.js

    refactor: move Cancel button to CompanyProfile components

src/components/CompanyProfile/CancelButtonWithConfirmation.scss
src/components/CompanyProfile/CancelButtonWithConfirmation.tsx
src/components/CompanyProfile/CancelButtonWithConfirmationContainer.tsx
src/pages/CompanyProfile/CompanyProfile.tsx

    feat: Employer - Company profile page Cancel button

src/pages/CompanyProfile/CancelButtonWithConfirmation.scss
src/pages/CompanyProfile/CancelButtonWithConfirmation.tsx
src/pages/CompanyProfile/CancelButtonWithConfirmationContainer.tsx
src/pages/CompanyProfile/CompanyProfile.tsx

In git log you can tell, that I finished feature, did some refactoring, and added integration tests afterwards. That was thoughtful workflow.

Can you tell the same story, by looking at example below?

$ git log --pretty="%n    %s" --name-only

    Changed scss

src/pages/CompanyProfile/CancelButtonWithConfirmation.scss

    Add Cancel button

src/pages/CompanyProfile/CancelButtonWithConfirmation.scss

    OK it doesn't work, forgot component. LOL

src/pages/CompanyProfile/CancelButtonWithConfirmation.scss
src/pages/CompanyProfile/CancelButtonWithConfirmation.tsx

    Tests

src/pages/CompanyProfile/CancelButtonWithConfirmationContainer.tsx
src/pages/CompanyProfile/CompanyProfile.tsx
cypress/integration/FunctionalTesting_Suite/CompanyProfilePage/company_profile_page.spec.js

    Is it working yet?

src/pages/CompanyProfile/CompanyProfile.tsx

Of course, you may not look at this without crying. You may end up, shamefully do git rebase to squash your commits to hide your crimes of uncertainty.

To start adopting commit guidelines, I would recommend to look at this project http://commitizen.github.io/cz-cli/

Code Review: Prereview checklist. No important tasks are forgotten.

Andrey Bodoev — Sun, 26 Apr 2020 06:26:36 +0000

Here's my rephrase on preflight checklist,

a prereview checklist is a list of tasks that should be performed by developer or team prior to raise an Merge Request

Everytime we discussing changes in code review, we might found ourself talking about same things over and over again. For example, asking in comments to add testing instructions or adding pivotal story number for reference.

To prevent that we need a little reminder to ourselves, or list of things we should check before raising an attention to your changes.

In our team we have checklists built-in in our Merge Request templates.

Here an example of one of our templates,

<!-- Make sure Merge Request title following "<type>: <subject> [#<pivotal_story_id>]" -->

<!-- Brief description. Examples are, screenshots, steps to reproduce, links to dependent MRs -->

### General Checklist:

- [ ] Exact versions in package.json
- [ ] Testing instructions?
- [ ] Docs? Examples are, update `README.md` file, or add ADR in `doc/adr`
- [ ] Tests?
- [ ] Put in copy at least two potential reviewers

/cc

/label ~"Review Me"

Structure of this template are self-explanatory, one thing I want to point out is last line: /label ~"Review Me" on which I already wrote an article.

And don't forget to treat checklists as living documentation, new items can be added, or removed throughout time.

It's purpose is to improve code review safety by ensuring that no important tasks are forgotten.

Code Review: Review labels in GitLab

Andrey Bodoev — Sun, 19 Apr 2020 12:27:24 +0000

We use GitLab to host our code. One of the aspects of it is deal with code reviews on daily basis.

In our team, we're about 10+ devs, it's quite difficult to have someone attention to code review, if it's product team and most of the time you focus on your own work and hoping if someone is review your changes.

Initially, we have this,

Here's an issue, which is no way to tell if someone is reviewing your code, or if you have enough reviewers and etc. Number of comments are not gave you much, and 0 of 2 is not really helpful here.

In our code review guidelines we have few rules, one of them is to have your Merge Request (abbr. MR) to be reviewed by at least 2 person in team.

There's few concerns to address,

how to make it visible if someone is reviewing your MR;
who reviewing what, and have they done so;
do you have enough reviewers.

Gitlab has it's own approval system in place, and code review guidelines. It does work, but we decided to tweak this a bit to improve communication within a team.

There's few types of labels we introduces,

Review Me, label is used to have an attention to MR and stating that it's ready to be reviewed, otherwise it can be wip;
Review by Paul Atreides, each reviewer has it's own label assigned once they start commenting;
Approved by Padishah Emperor Shaddam IV, each reviewer has it's own label once review is done, and comments are addressed.

With this in practice, once you going to visit list of MRs waiting. You're not checking each of them in order to find out, if it's been reviewed by someone else and it's ready to be merged. And it's greatly improve visibility of state on each Merge Request.

Code Review: Name file after exposed function

Andrey Bodoev — Fri, 10 Apr 2020 11:15:24 +0000

In code reviews which we conduct in our team, I might find some examples of code, which is can be misleading or with no clear intent. This post is about to share reasoning and hear critique on some of these examples.

Name file after exposed function

One of the cases is having few functions in one file, related or maybe not, or can be completely not related, but somehow group in one file.

get/
  index.ts
    export getSomething :: Int -> Int
    export getSomethingElse :: String -> String

Few things we can tell immediately,

please don't use index.js, https://www.youtube.com/watch?v=M3BM9TB-8yA&vl=en
both get directory and index.ts gave no context whatsoever.

Instead, try to name files after exposed function, and move each of them into it's own file, like so

getSomething.ts
  export getSomething :: Int -> Int

getSomethingElse.ts
  export getSomethingElse :: String -> String

It shows intent by looking at filename.

Few side effects which you may discover later,

Functions are enclosed in file to ensure there's no shared variables; which leads to proper unit tests and maintainability.
And after this change, there's powerful way to quickly go through project structure; by opening files which named after function and not searching through functions in search output of your text editor.

Takeaways

Name file after exposed function;
Breaking up into small modules for unit testing and maintainability;
Browsing code within file vs. browsing code within project.

One of the great examples in wild, https://github.com/lodash/lodash/tree/master/