DEV Community: Rishabh Garg

Effectively Reviewing Code

Rishabh Garg — Tue, 17 Oct 2023 17:01:02 +0000

YOUR JOB AS A REVIEWER IS TO UNLEASH THE PRODUCTIVITY OF YOUR TEAMMATES WHILE MAINTAINING QUALITY STANDARDS, READABILITY, AND THE INTELLECTUAL COHESION OF THE PROJECT.

Code reviews are an essential part of our engineering process. However, they can become onerous, especially with certain attitudes and dispositions by code reviewers. If approached with the wrong attitude, code review can be enervating and sometimes even downright counterproductive. If done wrong, it can be demoralizing and unnecessarily slow down everything. It's critical to approach the process in a way that maximizes value for everyone involved and minimizes cost and effort.

Reviewing Code is Not Writing Code

If there is one thing you remember from this guide it is this: the goal of a code review is not to make it so that the code looks as if you wrote it. The goal is to ensure that you agree on overall direction, to ensure that certain quality bar levels are hit, and – maybe most importantly – to serve as a vector of communication from the code writer to the code reviewer about what is going on.
Do not attempt to coerce the code writer to change the code such that it was written by you. Suggestions are fine; common style guidelines are fine; but sometimes you’ve got to show some level of flexibility if it wasn't written exactly the same way you would have written it.

Consistent Granularity

Reviews and commentary should be done on the same level of granularity. From a code author's perspective, it can be very frustrating to request feedback on the overall direction of a diff or a system, and have the review focus on details that do not matter at the moment. If you go through a diff and annotate a bunch of tactical changes and then at the end request changes and write something like // “Actually I think you should rethink this entire approach”//, that is a fail on the reviewer's part. It’s fine to rethink approaches, just don’t bombard the diff with tactical feedback that will no longer be relevant. An author might fix tactical changes as they go along only to find the bombshell that they have to start from square one.

A Nit is Not Enough

If your modifications and suggestions are nits and minor style issues, that is not a sufficient bar to request changes. Caring about nits is important. Pride in our collective work and making things pretty is important. You should care. But you shouldn’t allow it to unnecessarily slow things down either. Make your comments, note the nits, and accept the diff, for heaven's sake. Trust that your colleagues are respectful and that they will make the changes you wanted before commit or have a reasonable reason why they did not. If they frequently do not, that is their bad and you need to talk to them or even use our formalized feedback channels if necessary. If you don’t trust your colleagues, well then you and your team better work on that.
This does not mean that you shouldn't be detail-oriented, or that you shouldn't request changes based on details. "A nit is not enough" should not be translated to "A detail is not enough." Nits are non-controversial style issues that should probably be described in a document or be well-known as a convention within a team.
Notice! Be aware of the new Needs Final Review process
If you approve a diff with comments or nits, be aware that changes after approval will trigger the need for a "final review". In most cases this is not land-blocking for the author but will need to be completed in the next 10 days. In a few sensitive code paths the diff will actually require another approval after any changes before landing.
For more details see: Phabricator/CodeAuditing/Needs Final Review

Stop Typing and Have a Conversation

Avoid long-winded discussions on diffs whenever possible. Avoid the temptation to do a knock down drag out fight on a diff. If you are not reaching convergence quickly with a reviewer, don’t spend 20 minutes crafting your next response. Instead just take five minutes and go talk to them or set up a VC if they are working at another site. It almost always works better.
It is worth noting that long diff discussions are sometimes appropriate on system-wide changes with a lot of stakeholders -- PreparableXHP is a good example (see D142783). However this is the exception, not the rule.
For posterity, always register what is being discussed/agreed offline back to the diff.

Build Trust

Frequently the root cause of code review thrash is the issue of trust. If a lack of trust is involved, it does not matter how good the code is. You will probably have a hard time reviewing it. This is especially true when changing code that someone else owns. You can usually assume they trust you to be a good engineer and to make sane decisions. What you cannot assume though, is that they trust you to have all the knowledge they have when making those decisions.
So, build trust. The best way to do that is to talk. When starting the discussion be ready to listen and be open to change. Make sure that you understand their values, needs, context, and goals. Make sure that they trust that you understand that and will act accordingly. If you feel that the issue at hand is controversial, go into the discussion assuming that you're wrong. Try to listen and pay more attention to what they say rather than what you think is true. Bear in mind that often the simple act of listening and understanding helps more to build trust than a pile of awesome code. It's not that you shouldn't stand your ground. You should. Defend your decisions. They are as important as the other person's are, but not more. Explain what you are doing and why, so when the diff actually arrives they will have context.
If there are trust issues within a team, leverage your manager. Talk to them about it.
Trust is not built at once. It takes a few (often offline) interactions. Nothing builds trust better than being a good engineer to work with: write manageable diffs. Write good diff summaries. Write good test plans. Actually follow your test plans. If you do this consistently, people will trust you. You will move faster.

Ask Questions

The author is usually the expert, and has probably thought about the implementation in detail. Hence, don't be afraid to be wrong or sound stupid if you don't quite understand the diff. Chances are the author has provided insufficient context and explanations to the problem he/she is trying to solve. As reviewers, we need to fully understand the diff in order to provide useful suggestions. Phrase your suggestions as questions so that the author can evaluate the pros and cons instead of blindly implementing it.

Find people to emulate

The best way to learn how to review code effectively is to find people who do a good job and emulate and learn from them. It's a matter of taste that has to be developed over time. The best reviewers demand excellence respectfully, consistently focus on the most salient and important parts of a diff, and never unnecessarily slow you down. That is what you should shoot for.

Don't be afraid to put it back to the author's tray

There are reasons why the company has renamed this from "Request for Revision" to "Back to Author", one of which is to encourage reviewers to use this to get the author's attention. Context switching is hard, and authors usually want to address all the concerns at once instead of going back to the diff multiple times a day. So it helps for authors to know that the review is completed and they should check out their feedback and suggestions. At the same time, you may have questions that should be answered in order for you to continue reviewing. So notify the author of those questions by putting it back to their queue. Remember that requesting for the author's attention is not rude.

It's OK if you don't know

Identifying problems is usually easier than coming up with solutions. But don't let this stop you from highlighting why you think it's problematic in the first place. This could be as simple as “I don't like that class name but I have no ideas for a better name.” Provide reasons why the current solution is problematic, admit that you do not know of a better solution, and ask the author to think about the problem some more. Gives others a chance to see your perspective and perhaps, they can propose a better solution. You can offer to brainstorm together as well.

Final Review

This also isn't the "end," and you don't need to worry too much about whether your partner will actually "do it right," thanks to a new (well, new-ish as of August 2021) feature called "Final Review." Some truly important diffs that touch product code and have privacy implications will show up again for your "final review," allowing you to verify that the changes you requested when you approved the diff were really done. If not, you can raise a concern there and ensure that it really gets done. (Or you can just fix it yourself, or let them know -- all depending on the impact!)

Relocating from India to the UK: An Aspiring Engineer's Complete Guide

Rishabh Garg — Sun, 15 Oct 2023 10:19:26 +0000

As an Indian software engineer who relocated from Bangalore to London, I want to provide aspiring engineers with comprehensive insights on making this major transition. Whether you're a fresh graduate or experienced professional, moving abroad for new career opportunities can be life-changing. I'll share key details I wish I knew before embarking on my journey to the UK.

Navigating the Visa Process

For skilled workers, the Tier 2 (General) visa allows you to work in the UK if you have a job offer from a licensed sponsoring employer. You'll need to provide proof of your offer, English proficiency, maintenance funds, and salary above the minimum income threshold. The processing time can take up to 3 months.

Students often choose the Tier 4 (General) student visa, which requires you to gain admission into a recognized UK institution approved by the government. You'll need to provide proof of acceptance, maintenance funds to cover tuition and living costs, and appear for a TB test. This process can also take 2-3 months.

Having all required documents ready and meeting the visa conditions is crucial. Hiring an immigration lawyer can also help navigate any complexities in your situation.

Furthering Your Education

Pursuing a 1-2 year Master's degree in the UK has advantages like gaining specialized knowledge, honing technical skills, and getting access to research and job opportunities through campus networking. However, the job market can also be quite favorable for experienced Indian engineers without an advanced degree, so carefully consider your career goals, skills, and if certain roles require extra qualifications.

For example, if you have 3-4 years of industry experience in India, you may be able to find direct job opportunities in the UK by reaching out to recruiters on LinkedIn or applying directly through company websites. Referrals can significantly improve your chances of securing roles.

Salary and Cost of Living Differences

Engineers can earn substantially higher salaries in the UK compared to India, especially in senior engineering or managerial roles. For example, the average salary for a software engineer with 5 years of experience could be £55,000 in London, which is significantly higher than salaries for similar roles in India.

However, do factor in that the cost of living, especially housing, is extremely high in cities like London. Average monthly rent for a one-bedroom apartment in central London is £2,000 per month. Groceries, transportation, and other living expenses also add up. Make sure to budget accordingly and have realistic expectations of your savings potential.

Building a Social Network

Making new friends and building a social network is vital for your emotional well-being when relocating abroad. The UK is a multicultural society, so you can meet people through local community events, interest-based clubs, recreational sports teams, and volunteering activities.

If you're a student, universities organize orientation programs, socials, and cultural groups specifically for international students. This makes it easier to connect with fellow students. Outside university campuses, cities have diverse communities like Little India where you can find Indian grocery stores, restaurants, and make new friends.

Visiting Home

From London, I can travel home to Delhi twice a year which helps me stay connected with family and friends. Direct flights from London to Delhi take about 9 hours and cost ₹25,000-40,000 one way if booked a few months in advance. Try booking around major Indian festivals for cheaper fares.

The close proximity and multiple daily flight options make visiting India relatively convenient. Just plan your schedule accordingly based on work allowances. Going back home for 1-2 weeks allows me to replenish myself emotionally and physically.

Switching Jobs

One advantage of being in the UK on a sponsored skilled worker visa like Tier 2 is that you can switch jobs just like you would in India. All you need is your new employer's sponsorship approval. They provide a new Certificate of Sponsorship that you use to apply for an updated work visa.

For those who came to the UK as students on a Tier 4 visa, you can switch to the Post-Study Work (Graduate Route) visa after graduation. This 2-3 year visa lets you work or look for employment in any field without requiring sponsorship. It makes switching between companies hassle-free.

Dealing with Racism

While the UK is considered more inclusive than some other countries, isolated incidents of racism or discrimination do unfortunately occur. Being aware of support networks like employee resource groups (ERGs) can help you find community and allies in your workplace.

Seeking help from institutions like UKCISA if faced with any harassment as a student is also advised. Most people's experiences are positive, but reporting any discrimination you face is important. Keep your support system strong during challenging times.

Relocating abroad requires courage and adaptability. With the right preparation, Indian engineers can thrive living and working in the UK's fast-paced multicultural environment. Let me know if you have any other questions!

Kotlin best practices

Rishabh Garg — Wed, 04 Oct 2023 07:02:00 +0000

"With great power comes great responsibility"

Kotlin is a modern programming language with many benefits and a lot of convenient and powerful features. However, some of these features come with unexpected performance costs that we should be aware of and careful with.

General guidelines

Do NOT :

Use Hungarian notation.
Use apply, let, run, etc unnecessarily, they reduce code readability. Usually unnecessary, but one good case is using them to avoid an explicit null check (e.g. x?.let { ... }).
Use String.format("%d", x), Kotlin supports inlining variables in strings directly: "$x".
Create private vars with a getter function. Instead, make a public var + privateset
Use by lazy initialization. It has significant hidden performance costs. See Lazy initialization for more details.
Use !! or checkNotNull unless required for legacy Java code (e.g. during Kotlin conversions, if using legacy Java APIs, etc.). It's one of the only ways to get a NPE in Kotlin. There is always a better way to organize the code to avoid nullability, or to use the Kotlin safe operator (?.).
Use Dispatchers.Default orDispatchers.IO. Instead, use our own CoroutineDispatchers.Background, the Kotlin ones use a different thread pool.
Use runBlocking or backgroundRunBlocking, they will block the underlying thread causing performance issues.
Create functions that take in lambda function arguments, without making the higher-order function inline. Not using inline can significantly affect performance. See Higher-order functions for more details.
Mix Kotlin and Java in the same BUCK module. This is supported, but it can cause subtle bugs because of Java-Kotlin interoperability.

I pulled some of the above don'ts from a list of somewhat outdated dos and don'ts in this Stella Kotlin guide. Do take a look at the 'Common Patterns' section of that guide for some useful ways to avoid some of the patterns above.

Companion objects

Companion objects provide an easy way to add static functions and variables to a class.

There used to be additional overhead introduced by companion objects, but it has since been optimized by Redex so you are free to use them.

Use cases and best practices:

Constants (const val)
- Private constants (e.g. TAG strings): no need for a companion object, you can just define them outside the class.
- Public constants: prefer placing them inside companion objects, so that when accessed from other files they are scoped to the class they are defined in.
Static functions
- If all functions are static, use object instead of class. Objects are equivalent to Java singletons, and are instantiated lazily on first access.
- Otherwise place them inside a companion object (and add @JvmStatic if called from Java code)

Beware Kotlin objects

Kotlin objects follow the singleton pattern. While these can be useful, we should never create stateful Kotlin objects:

They can cause memory leaks given that their lifecycle is scoped to the application. For example, storing a Context variable in a Kotlin object can very easily leak an entire activity.
They are very hard to unit test. This is because state is hard to reset between test runs, and dependencies are impossible to inject without workarounds.

Instead:

Only create Kotlin objects to store static functions and constants.
If you really need a singleton, create it manually – make your singleton a class and then store the singleton instance in a companion object, injecting any dependencies. Do not store Context variables or any other component that is not scoped to the application lifecycle. See example.

Lazy initialization

In Kotlin you can initialize a variable lazily / on first access using the lazy keyword. This is very convenient, but when compiled down to Java there is a lot of unexpected additional overhead. Read the Lazy Initialization section of this wiki for a detailed explanation on this.

As a general rule, don't use it. However, the overhead can be worth it if the initialization of an object is particularly expensive and there's no other way to organize the code.

Higher-order functions and Lambda expressions

Kotlin supports lambda expressions and storing functions in variables:

val sum = { x: Int, y: Int -> x + y }

You can also pass functions into other functions:

fundoOperation(a: Int, b: Int, operation: (Int, Int) -\> Int) 
{
return operation(a, b)
 }

We can now pass sum into doOperation
doOperation(2, 3, sum)

You can also directly pass the lambda expression to the higher-order function:

doOperation(2, 3) { x, y -\> x + y }

This is very useful to reduce boilerplate and improve code readability, but it comes at a performance cost. There is additional runtime overhead because each function argument becomes an object capturing a closure (the group of variables accessed in that function), requiring extra memory allocations and virtual calls.

To solve this, Kotlin offers the inline keyword:

inlinefundoOperation(a: Int, b: Int, operation: (Int, Int) -\> Int) {
return operation(a, b)
 }

This inlines the function argument inside the higher-order function, removing the additional overhead. Note that this will increase code size, so as a general rule avoid inlining large functions.

Writing Good Alerts: How to Create Monitoring That Actually Helps Humans

Rishabh Garg — Wed, 04 Oct 2023 06:31:03 +0000

Alerts. Those pesky notifications that interrupt your dinner, jolt you awake at 3am, or flood your inbox with false positives. I feel your pain. But robust monitoring is crucial for catching issues early. The key is designing alerts that help humans manage systems, not overwhelm them.

Here are some tips I've learned for creating effective, human-centric alerts:

1. Ensure alerts lead to concrete actions

A good alert should enable responders to quickly mitigate issues or escalate them. Ask yourself:

Are the metrics clear indicators of a specific problem? Vague metrics lead to confusion.
Does the alert contain enough context like IDs, snippets, or charts to diagnose root cause? Details speed investigation.
Are runbooks provided for triage, debugging steps, and contacts? Documentation aids responders.

2. Set dynamic, tailored thresholds

Static thresholds fail as systems evolve. Instead, consider:

Adjusting thresholds based on trends, seasonal usage patterns, and new behaviors. Weekends may differ from Tuesdays.
Updating error budgets when introducing new failure scenarios. Additional fallback logic impacts counts.
Tightening thresholds initially for new alerts, then relaxing once confident they work.

3. Use rigorous prioritization

Not every anomaly requires waking someone up. Thoughtfully prioritize based on:

Severity of the incident. Reserve high priority for true emergencies needing immediate response.
Likelihood it's a transient, self-healing glitch. Don't stress responders unnecessarily.
Relevance of the error domain. Route alerts to appropriate teams to avoid distraction.

4. Continuously improve alerts

Analyze false positives, false negatives, feedback and tagged issues to iteratively refine alerts:

Identify faulty metrics or thresholds by reviewing incorrectly triggered alerts.
Surface missing alerts by correlating outages with alert gaps.
Incorporate pain points and ideas from oncall engineers through surveys.
Fix problematic alerts tagged with "noisy alert" or similar keywords.

Thoughtfully designed, continuously refined alerts provide responders valuable insights, not endless distractions. Now instead of battling alert noise, they can focus on quickly solving real problems. That's monitoring done right.

What strategies have worked for you when creating human-centric alerts? Share your wisdom below!

Get It Done: The Power of Leveraging Leadership

Rishabh Garg — Wed, 04 Oct 2023 06:18:13 +0000

We've all been there - struggling to accomplish an important goal against seemingly impossible odds. As the person responsible, your instinct is to put your head down, work harder, and "do the best you can."

But here's the secret: that's NOT always the right approach. When major roadblocks threaten mission-critical objectives, good leadership wants to know. Your mandate isn't just to try your hardest - it's to get it done. And if that requires asking for help, you should do so without hesitation.

I understand the reluctance. We don't want to bother senior leaders with problems. We worry it makes us look incompetent. But strong leaders want more than silent struggle - they want transparency around obstacles so they can unblock progress. Some truths I've learned:

Involving leadership early builds trust. Leaders would rather help you course correct than explain failure after the fact. Being upfront when things go wrong demonstrates courage and accountability.

Leaders can rapidly break deadlocks. A leader clarifying priorities may achieve in minutes what teams debate for months. Avoid wasted cycles and act quickly when alignment issues arise.

Not every problem is easy to solve. Looping in leadership isn't admitting failure - it's getting the right people involved. Complex issues may need more seniority, authority or different skills.

Asking for help sets realistic expectations. If leaders understand the true difficulties, they can adjust goals and timelines rather than assume results that may be impossible.

Of course, you want to equip your team to operate autonomously as much as possible. But in an imperfect world, obstacles happen. When they threaten key goals, get the right people involved immediately. Move egos aside and do whatever it takes to get it done, even if that means speaking up.

Strong leaders want to hear the unvarnished truth so they can course correct. The courage to promptly voice concerns and ask for help demonstrates leadership in action. So when major barriers arise, don't stay silent - get leadership engaged. Progress may be just a conversation away.

What do you think? When have you seen involving leadership unblock stalled initiatives? Share your stories below!

How to have effective oncalls

Rishabh Garg — Tue, 03 Oct 2023 13:27:09 +0000

Surviving Oncall: Tales from the Incident Trenches

Ah, oncall. Those late night alerts that jolt you awake, summoning you to battle unexpected bugs and outages. Thrilling? Sometimes. Stressful? Often. Manageable? Absolutely - with the right preparation and mindset.

Let me share some hard-earned tips to help you survive and thrive during your oncall tours of duty. Think of me as your wise, veteran software knight passing down battle-tested strategies.

Before Your Watch

Set up call forwarding - Make sure your phone is set up to receive calls even if you're on Do Not Disturb. You don't want to miss that 3am alert!

Review past incidents - Read summaries from previous oncall shifts to be aware of any ongoing or recurring issues.

Limit non-urgent work - Try not to schedule tasks that require heavy focus like launches or complex debugging. Handle those outside oncall weeks when possible. Oncall is for fighting fires, not building new castles.

Prioritize tech debt - Use oncall weeks to pay down eng debt through tests, automation, and documentation. This makes future oncalls smoother.

During Incidents

Triage alerts - Check alert docs, failing timeseries, and charts to understand the scope and root cause. Swift action contains damage

Debug efficiently - Leverage tools group-bys, CodeHub to quickly identify issues.

Escalate appropriately - Involve relevant teams early if you suspect the problem is not isolated.

Communicate proactively - Keep stakeholders updated on SEV status and next steps through SEV tools and chats.

Support team members - Have empathy for those involved in an incident. Avoid finger pointing.

After Your Watch

Write summaries - Document oncall incidents, learnings, and follow ups while memory is fresh.

Attend debriefs - Reflect on opportunities to improve documentation, alerts, tests, and overall oncall process.

Handoff open issues - Ensure incoming engineer is aware of any ongoing investigations that need follow up.

Rest and recover. Heroes need downtime before the next battle

Following structured oncall best practices goes a long way in reducing stress, minimizing fire drills, and strengthening team resilience. With the right preparation, diligence during incidents, and thorough follow up, you can handle oncall with confidence. Now grab your laptop, we've got outages to fight!