|
prom candy posted:Edit: ^^^ I like PRs because I like people to have a view into what I'm working on and having a view into what they're working on. We get this with sprint planning/scoping, transparency between teams via lots of Slack updates about what’s happening (each big service has a channel for questions, and issues often get generated from these), short daily standups and a meeting every other week to talk high level stuff with all teams. Anyone can see any teams’ work in progress. We’re careful to break tasks down into small chunks so we can deploy a lot and get quick feedback. Like I said I’m quite green and there are obviously tradeoffs. As someone who changed careers and enjoys lots of small accomplishments, it works better, and I can learn faster. It’s more of a restaurant kitchen than the glacially paced corporate offices I’ve been part of. I do notice the less gregarious new hires and people who want to be told what to do don’t seem to like it as much. To be fair, I only spent a brief internship with the alternative. Hence sitting on my hands waiting for approval.
|
#
?
Jul 9, 2023 21:53
|
|
|
|
| # ? May 10, 2024 13:09 |
|
|
DkHelmet posted:My last gig had the senior devops engineer not know how memory works on Linux What's this mean? I'm really curious. Tell me a horror story!
|
|
#
?
Jul 9, 2023 22:10
|
|
|
Doktor Avalanche posted:shouldn't feature branches be made from main since it's the latest "clean" branch (fully or at least almost fully tested and working) Probably yes. I think the idea is sometimes the previous feature would depend on the next one? It's not a very good system.
|
|
#
?
Jul 9, 2023 22:18
|
|
|
Doktor Avalanche posted:shouldn't feature branches be made from main since it's the latest "clean" branch (fully or at least almost fully tested and working) No, if you did this, you'd run into a poo poo show when you merge it into dev, unless that part of dev hadn't been changed and then why didn't you just branch off of dev? There's also the (mostly bullshit) philosophy that things should only go into main. ThePopeOfFun posted:To be fair, I only spent a brief internship with the alternative. Hence sitting on my hands waiting for approval. PR approvals are a big place to be picky on interns to get them to have Good Habits, so your experience may not have been the best. I use GitFlow because all our stuff involves some effort to release to the customer, so we do that on merges to main.
|
|
#
?
Jul 9, 2023 22:19
|
|
|
cum jabbar posted:What's this mean? I'm really curious. Tell me a horror story! Devs report slowness in a prod system, he ssh'd in and ran free. He noticed that it was near 95% used, so he doubled the instance size. Reboot, ssh in, notice it's at 95% again, redouble instance and reboot. He wasn't aware that linux uses buffers and will greedily make use of ram, and didn't know the difference between free and available. Dude never heard of memory pressure and PSI either. The underlying slowness was a long running query with no indexes over yonder in RDS. What do you want for $185k/year? You'll note what I said above about low competency orgs and git. This place had a "devops" department with cattle over 10 years old. First tool in the box is SSH.
|
|
#
?
Jul 9, 2023 22:22
|
|
|
All code should be reviewed by someone other than the person who typed it before it makes it back to the trunk, but that is fully doable even in the most extreme "commit directly to the trunk" workflow.
|
|
#
?
Jul 9, 2023 22:34
|
|
|
ThePopeOfFun posted:No pull requests rules. The Senior with Opinions About Whitespace cannot waste his time on approvals. I can imagine a terrible scenario where my coworkers deployed poo poo code and hated speaking to each other. Trunk based would not work there. As a work life, heavy scrutiny and pulls requests are nice if you're anxious, new or want a chill job doing what you are told. On the other hand, trunk based forced my still very green self to think through problems, test, and ask for help if I needed it before deploying. I personally enjoy a faster pace. I found sitting on my hands between approvals and having to bug whoever was on approval duty grating. Ye gods I cannot imagine how awful going without PRs would be. For starters, you should be using a linter of some kind as part of your workflow to avoid any sort of arguments about formatting bullshit - you can argue about the linter config as a separate topic, but once it's in there you just run it and what it approves makes it in. Once you get past that you cut away the vast majority of totally useless feedback. That being said, waiting on PRs does suck, but a lot of that should probably come down to 'prioritize reviews'. Bongo Bill posted:All code should be reviewed by someone other than the person who typed it before it makes it back to the trunk, but that is fully doable even in the most extreme "commit directly to the trunk" workflow. This. If I write code other people can't understand at all, that's probably a problem, since otherwise I'm going to gently caress up their day when they have to fix it later, and lord knows I don't want to be singularly responsible for bugfixes. Edit: Actually, wait, maybe I haven't done trunk based dev, typically I work on a task on my own for a couple days or whatever, handling it locally, and then merge directly to mainline. https://trunkbaseddevelopment.com/ seems to indicate that everyone commit at least once every 24 hours to mainline, which seems...bizarre, compared to just committing something as a solid coherent piece of code. We don't do feature branches or anything though, it just tends to be like 'one task = one bit of shared code', and we all merge straight to mainline after a PR. Falcon2001 fucked around with this message at 22:55 on Jul 9, 2023 |
|
#
?
Jul 9, 2023 22:45
|
|
|
Honestly, in theory there isn’t much of a problem with having PRs that don’t require any human approvals and just need to pass the checks to get merged. All it takes is one person taking a shortcut like disabling the checks to ruin it though.
|
|
#
?
Jul 9, 2023 23:09
|
|
|
Falcon2001 posted:Ye gods I cannot imagine how awful going without PRs would be. For starters, you should be using a linter of some kind as part of your workflow to avoid any sort of arguments about formatting bullshit - you can argue about the linter config as a separate topic, but once it's in there you just run it and what it approves makes it in. Once you get past that you cut away the vast majority of totally useless feedback. Yeah I was gonna say that senior should probably think about linter and a githook or something. Doesn't sound very senior to me 
|
|
#
?
Jul 9, 2023 23:12
|
|
|
Huh, the guy who advocated trunk-based development here framed it as an alternative to PRs. Commit directly to main, CI ensures that nothing broken gets deployed. Implement a feature flag first and use TDD for everything. Replace code reviews with continuous pair programming. But listening to you all we are pretty close to trunk-based development with our actual practices. We do short (most of the time) feature branches to dev and release weekly by merging to main. But we treat dev as always-releasable and it's probably been a year or more since it wasn't. So we are thinking of getting rid of it.
|
|
#
?
Jul 9, 2023 23:33
|
|
|
If you do real trunk-based development then yeah I don't think you use PRs at all. They only come into place if you'd be doing like feature branches or whatever which have been pretty common across all companies I've been employed at If you're committing straight into the trunk branch then you probably have a decently small team working on it and can keep each other in the loop another way. And you might be committing infrequently enough at that point you just do stuff on commits instead of PRs Macichne Leainig fucked around with this message at 23:41 on Jul 9, 2023 |
|
#
?
Jul 9, 2023 23:37
|
|
|
Falcon2001 posted:Edit: Actually, wait, maybe I haven't done trunk based dev, typically I work on a task on my own for a couple days or whatever, handling it locally, and then merge directly to mainline. https://trunkbaseddevelopment.com/ seems to indicate that everyone commit at least once every 24 hours to mainline, which seems...bizarre, compared to just committing something as a solid coherent piece of code. The point is not to have a time limit on your commit, or scheduled commits or anything. They're just pointing out that the branch is meant to be short lived and limited in scope. It's not a total disaster if you leave for the day without merging your branch, but ideally you will merge multiple times during a workday. Like, the nightmare scenario is the person who spends three weeks working on some refactoring and then sends you a pull request touching 150 files.
|
|
#
?
Jul 9, 2023 23:44
|
|
|
Pull requests are a convenient way to get a review when the work is remote. You might create a "branch" consisting only of a single commit with a single unit to be reviewed. If you keep the commits small (because the user stories are small, as they should be) then it's basically still trunk-based.
|
|
#
?
Jul 10, 2023 00:03
|
|
|
Falcon2001 posted:Edit: Actually, wait, maybe I haven't done trunk based dev, typically I work on a task on my own for a couple days or whatever, handling it locally, and then merge directly to mainline. https://trunkbaseddevelopment.com/ seems to indicate that everyone commit at least once every 24 hours to mainline, which seems...bizarre, compared to just committing something as a solid coherent piece of code. it's worth noting that site like that are just some dudes opinion, that there is no one size fits all solution, and you should look to other peoples work to avoid making the same mistakes they did but do what works for you
|
|
#
?
Jul 10, 2023 00:08
|
|
|
Like, following that practice, if I'm refactoring existing code, and at the end of day I did about half of the work, which now causes the code to no longer compile because some of the function calls don't match the function signatures anymore, am I supposed to make a PR with both the original code, and an incompletely refactored copy of the code that is commented out so that the compiler doesn't complain? And where the reviewer can't make sense of my design because it's not complete yet? That sounds stupid to me.
|
|
#
?
Jul 10, 2023 07:07
|
|
|
Carbon dioxide posted:Like, following that practice, if I'm refactoring existing code, and at the end of day I did about half of the work, which now causes the code to no longer compile because some of the function calls don't match the function signatures anymore, am I supposed to make a PR with both the original code, and an incompletely refactored copy of the code that is commented out so that the compiler doesn't complain? And where the reviewer can't make sense of my design because it's not complete yet? Yeah this is confusing to me; A lot of times my code just straight up doesn't work when I stop for a given day, or would break some poo poo other than specific code flows.
|
|
#
?
Jul 10, 2023 07:15
|
|
|
Carbon dioxide posted:Like, following that practice, if I'm refactoring existing code, and at the end of day I did about half of the work, which now causes the code to no longer compile because some of the function calls don't match the function signatures anymore, am I supposed to make a PR with both the original code, and an incompletely refactored copy of the code that is commented out so that the compiler doesn't complain? And where the reviewer can't make sense of my design because it's not complete yet? No, of course not. The argument is that if it takes multiple days to complete the task you're working on, it should ideally be broken down into smaller tasks that can be merged atomically. One of the solutions proscribed for developing features where this is not possible is to use feature flags. thotsky fucked around with this message at 07:20 on Jul 10, 2023 |
|
#
?
Jul 10, 2023 07:16
|
|
|
The pull requests should be complete units of work by a single developer, not half-finished garbage. You just don't send a pull request if you haven't finished that unit of work, instead you'll finish it up and send the PR tomorrow. Sometimes you'll send multiple PRs, if you finished multiple units of work on that day. If your units of work are typically taking way more than a day to finish, that's a sign that you're not breaking them up enough. For example, instead of submitting a big refactoring as a single change, you could do it as several consecutive changes: - Introduce a new, revised API - Migrate callers to use the new API (this could be several different PRs, e.g. so you could get reviewers who are subject-matter experts in the code you're migrating) - Delete the legacy API
|
|
#
?
Jul 10, 2023 07:18
|
|
|
Any time I did a change to our code generation tools back at my previous job, I liked rewriting the commits for the PR so there'd be one for the changes to the tool and a separate one for regenerating the generated code. Then sometimes there'd be additional commits to revise stuff to take advantage of whatever new feature I added. I have no idea if anybody actually appreciated this while reviewing the PR, but it sure made my inevitable "git blame" six months later a lot cleaner!
|
|
#
?
Jul 10, 2023 07:30
|
|
|
amazed at everyone itt who are in favour of making even more tickets
|
|
#
?
Jul 10, 2023 07:40
|
|
|
Jabor posted:The pull requests should be complete units of work by a single developer, not half-finished garbage. You just don't send a pull request if you haven't finished that unit of work, instead you'll finish it up and send the PR tomorrow. Sometimes you'll send multiple PRs, if you finished multiple units of work on that day. Possibly it's that I've been working on stuff lately where it's like 'add X brand new thing, that has 5 subthings', and I guess I could just commit code that doesn't do anything yet, but it was taking us around 4-5 days to finish one of these. I guess it would push you to make changes that are very untouchable at first (a Class that never gets called but is built), but that feels like it would in turn make your tests harder to write for certain scenarios. Like for example, my latest thing I worked on was a click CLI program so you'd use the click CLI test runner, which in turn means you'd have to have at least a basic working class tied into the main CLI loop. I guess the answer to that is just 'well don't run that', and don't auto-promote every build to release? This would turn every fairly straightforward task into like eight more and I don't really see that being beneficial ( And I don't really mind having tasks to track meaningful work either, I use tickets as my main documentation on what I'm doing). I guess if my team all did this I'd probably just shrug and go along with it, but still, feels weird.
|
|
#
?
Jul 10, 2023 07:53
|
|
|
If you can't convince me to make atomic commits how do you plan to sell me on... uh, diatomic molecular PRs instead of my buckyballs that take weeks to craft?
|
|
#
?
Jul 10, 2023 07:58
|
|
|
None of this will be a problem when Autopilot can make and accept the PRs for us.
|
|
#
?
Jul 10, 2023 08:02
|
|
|
Perhaps a concrete example will help clarify things here: We divide the different builds of our app into environments, and every environment can be built from trunk at any point. To determine what code actually goes into each environment, we use feature flags. We have a big file that lists all our flags and what environments they're enabled in, and that gets transformed into an environment-specific list of bools for compiling the actual app. (This allows the compiler to do dead-code elimination and entirely strip out code that's only used for disabled features). When you're building a new feature, you create a flag for it, which is initially switched off everywhere. You and your team switch it on in your own local builds when you're working on that feature, and it can be in whatever state of unreadiness you like as long as it actually compiles. When you have it ready enough that it's not going to disrupt any other developer's workflow, you make a PR that switches that feature flag on for the dev environment. You can keep iterating and building it up at this point, but every change has to keep it at a "won't disrupt other developers" level of working. Once you are feature-complete, you make another PR to flip the flag on in internal testing environments. Once it's been QA'd and approved for release, another PR flips it on everywhere.
|
|
#
?
Jul 10, 2023 08:47
|
|
|
thotsky posted:ideally you will merge multiple times during a workday. That sounds really loving exhausting op. We just use the "no stupid bullshit" model. Development happens against main, releases are made as tags (and if hotfixes are needed, branches) on main when we decide to cut one, to merge your code you open up a PR that needs human review + automatic checks to pass. PRs are open as some reasonable amount of work is done.
|
|
#
?
Jul 10, 2023 08:47
|
|
|
Xarn posted:We just use the "no stupid bullshit" model. Development happens against main, releases are made as tags (and if hotfixes are needed, branches) on main when we decide to cut one, to merge your code you open up a PR that needs human review + automatic checks to pass. PRs are open as some reasonable amount of work is done. That's not incompatible with what I was saying, it's just that "some reasonable amount of work" is more often measured in hours rather than days.
|
|
#
?
Jul 10, 2023 10:07
|
|
|
redleader posted:amazed at everyone itt who are in favour of making even more tickets It's no big deal if you're not using Jira, or, failing that, if you're using a Jira instance that has been configured in such a way as to turn off as much bullshit as possible.
|
|
#
?
Jul 10, 2023 10:31
|
|
|
Jabor posted:Once you are feature-complete, you make another PR to flip the flag on in internal testing environments. Once it's been QA'd and approved for release, another PR flips it on everywhere.
|
|
#
?
Jul 10, 2023 13:08
|
|
|
Ultimately, it's done through a culture of cleaning up your feature flags once you've shipped the feature they were guarding. (This culture is encouraged through automated tooling that e.g. files a cleanup ticket once the flag has been switched on everywhere for a certain length of time). Since each team is only working on a limited number of features at any given time, this helps avoid scenarios where you have to keep track of a combinatorial explosion of overlapping flags. The process of removing feature flags is quite rote and mechanical, you just go to everywhere the flag is referenced and delete the branch that is now never taken. Unit tests are handled through some voodoo magic: When compiling for unit tests, the boolean flags are non-const, so all the code is kept and the tests can set the flag appropriately for the scenario they're testing. Typically you don't need to change unit tests at all when removing a flag, just delete the ones that were only testing the code you've now deleted.
|
|
#
?
Jul 10, 2023 14:05
|
|
|
thotsky posted:No, of course not. The argument is that if it takes multiple days to complete the task you're working on, it should ideally be broken down into smaller tasks that can be merged atomically. One of the solutions proscribed for developing features where this is not possible is to use feature flags. A common obstacle I’ve encountered for this is who does the task of breaking things down into smaller tasks. Hell, most places struggle to provide coherent stories, and then those coherent stories are often too large and the only people with the capability to decompose them are the devs. And then devs do that instead of writing code.
|
|
#
?
Jul 10, 2023 14:35
|
|
|
redleader posted:amazed at everyone itt who are in favour of making even more tickets loving seriously. also the attitude that a bunch of current best practices aren't going to end up being considered "dumb poo poo that we did five years ago because we didn't know better" or on a longer-term scale how stuff like OOP was considered the wave of the future for years and years. yet now there are places where inheritance is a swear-word.
|
|
#
?
Jul 10, 2023 14:48
|
|
|
Jabor posted:Ultimately, it's done through a culture of cleaning up your feature flags once you've shipped the feature they were guarding. (This culture is encouraged through automated tooling that e.g. files a cleanup ticket once the flag has been switched on everywhere for a certain length of time). Since each team is only working on a limited number of features at any given time, this helps avoid scenarios where you have to keep track of a combinatorial explosion of overlapping flags. i have never removed a feature flag in my entire life.
|
|
#
?
Jul 10, 2023 14:50
|
|
|
it’s another software development practice in the category of “anything works well when there’s 10 total devs and they’re all good at their job and friendly and we make a thing where if it breaks it’s a minor inconvenience and also we have zero cost to the user updates”
|
|
#
?
Jul 10, 2023 15:03
|
|
|
Another wasted work day because the box I’m currently developing for is broken and the infrastructure team still hasn’t responded to my request for a power cycle since Friday. 
|
|
#
?
Jul 10, 2023 15:40
|
|
|
Lyesh posted:also the attitude that a bunch of current best practices aren't going to end up being considered "dumb poo poo that we did five years ago because we didn't know better" I just know I prefer it to what I was doing before, but if something better comes along I will use that.
|
|
#
?
Jul 10, 2023 15:43
|
|
|
redleader posted:amazed at everyone itt who are in favour of making even more tickets New tickets are always going to show up. Might as well have some control over what kind they are
|
|
#
?
Jul 10, 2023 15:50
|
|
|
For PRs how do you handle a scenario where you have a bunch of tickets that are pretty closely related? Like for example today I have to add a screen which is a list of things and then there are a few extra tickets for simple things that you can do with each list item. My preference would be to batch these all into a single PR because otherwise I'm going to wind up waiting around for approval between each one, because one step is dependent on another. Would you just stack like 3-4 tickets into a single PR if the changes are simple enough? The reason they're 3-4 tickets is because when we get into it QA and Design Review it's annoying to have one larger ticket getting kicked back and forth and keeping track of the requested changes.
|
|
#
?
Jul 10, 2023 16:04
|
|
|
Anyone have a good source on feature flags and database migrations/reversions? This was always the hardest part for me to deal with and one of the reasons we don't use feature flags where I work.
|
|
#
?
Jul 10, 2023 16:08
|
|
|
DkHelmet posted:Devs report slowness in a prod system, he ssh'd in and ran free. He noticed that it was near 95% used, so he doubled the instance size. Reboot, ssh in, notice it's at 95% again, redouble instance and reboot. He wasn't aware that linux uses buffers and will greedily make use of ram, and didn't know the difference between free and available. Dude never heard of memory pressure and PSI either. The underlying slowness was a long running query with no indexes over yonder in RDS. *reads this post* *checks notes for own job title* *furiously begins googling about linux memory* At least I know how to horizontally scale in AWS.
|
|
#
?
Jul 10, 2023 16:08
|
|
|
|
| # ? May 10, 2024 13:09 |
|
|
redleader posted:amazed at everyone itt who are in favour of making even more tickets But me filing tickets, bugs, and blockers is the only way my manager sees that I’m doing any work at all!!! (despite me constantly updating the team at standup with my current status  )If I don’t make tickets clearly I don’t exist. Lyesh posted:loving seriously. Yeah I’m fine with everything going away eventually. It’s not a problem if we eventually decide “oh this sucks actually and we shouldn’t be working like this” because learning that something won’t work out is just part of revealing the truth of things.
|
|
#
?
Jul 10, 2023 16:11
|
|