|
Pollyanna posted:
For bug fixes, it's easiest just to look at the final diff. For larger features, though, the commit history shows a lot about the design process, and it can be handy to look at it.
|
#
?
May 8, 2017 16:37
|
|
You don't have to comment as you step through, I just wait until the end before I bring up stuff that didn't eventually get addressed. Granted, I do this mostly for large scale changes instead of relatively minor things, so maybe it's different in that case.
|
|
| # ? May 14, 2024 12:22 |
|
|
I made a few pull requests where I suggested that the reviewers may want to look at one commit at a time, because the change was a big refactor that touched a few hundred files. The commits broke down to something like 1) change the code generator, 2) regenerate a bunch of code, 3) remove the now-unused functionality. I don't know if anybody took the advice and only looked at commits #1 and #3, or what.
|
|
#
?
May 8, 2017 16:42
|
|
|
Gounads posted:Bitbucket has an open to auto-squash on merge. Full commit history during code review and readable commit history on master. Is this just bitbucket cloud? I can't find that option for bitbucket server. We are using 4.10 though. We're behind on updating :/
|
|
#
?
May 8, 2017 18:08
|
|
|
geeves posted:Is this just bitbucket cloud? I can't find that option for bitbucket server. We are using 4.10 though. We're behind on updating :/ Our stash server does it. I'm not 100% sure how stash and bitbucket server are related. (just versions?)
|
|
#
?
May 8, 2017 18:13
|
|
|
geeves posted:Is this just bitbucket cloud? I can't find that option for bitbucket server. We are using 4.10 though. We're behind on updating :/ It's a recent feature and we're using it on our local bitbucket server. Talk to your admin.
|
|
#
?
May 8, 2017 18:27
|
|
|
Gounads posted:Our stash server does it. I'm not 100% sure how stash and bitbucket server are related. (just versions?) Bitbucket is just rebranded Stash as of a year or so ago. BabyFur Denny posted:It's a recent feature and we're using it on our local bitbucket server. Talk to your admin. I am an admin 
|
|
#
?
May 8, 2017 20:53
|
|
|
i like one commit per pr, whether you get it from rebasing or from squashing on merge. that way the thing that was reviewed is the thing that was merged and there's no opportunity to revert part of a merge or whatever
|
|
#
?
May 8, 2017 21:24
|
|
|
geeves posted:Bitbucket is just rebranded Stash as of a year or so ago. Good news, you don't actually have to upgrade: https://www.atlassian.com/blog/bitbucket/new-features-bitbucket-4-9
|
|
#
?
May 8, 2017 21:28
|
|
|
CPColin posted:I made a few pull requests where I suggested that the reviewers may want to look at one commit at a time, because the change was a big refactor that touched a few hundred files. The commits broke down to something like 1) change the code generator, 2) regenerate a bunch of code, 3) remove the now-unused functionality. I don't know if anybody took the advice and only looked at commits #1 and #3, or what. I do this a lot of the time too. I also remind people to set their diff tool to ignore whitespace because that often makes changes look less intimidating.
|
|
#
?
May 8, 2017 23:17
|
|
|
the talent deficit posted:i like one commit per pr, whether you get it from rebasing or from squashing on merge. that way the thing that was reviewed is the thing that was merged and there's no opportunity to revert part of a merge or whatever That makes commit messages entirely unhelpful, though. I'm not advocating that you leave the random "WIP" messages, but if a PR is only one commit and it touches a bunch of different things how in the world do you know if the work is appropriate? At current job, on my team, we do light-weight PRs which give you the nice advantage of not having to worry about too many commits (ours average 2-4) and keeping the work scoped to very small changes.
|
|
#
?
May 9, 2017 13:10
|
|
|
Blinkz0rz posted:That makes commit messages entirely unhelpful, though. I'm not advocating that you leave the random "WIP" messages, but if a PR is only one commit and it touches a bunch of different things how in the world do you know if the work is appropriate? Because usually there is more text related to the PR (like related tickets/issues, and description field in the PR itself) than the commit message, especially if you strongly adhere to the < 100 characters (or whatever was the number) for commit messages. Not to mention that you can just ask why a thing was done in a certain way and then discuss the implementation in the PR before merging.
|
|
#
?
May 9, 2017 18:36
|
|
|
HardDiskD posted:especially if you strongly adhere to the < 100 characters (or whatever was the number) for commit messages.
|
|
#
?
May 9, 2017 19:19
|
|
|
Yes, that was for the first line. And if your org works by going all ham in the commit description, like the unix (or git?) greybeards that have to e-mail diff patches around instead of relying on the tools that the community uses at large, then do it. On that point, I am now having the impression that people only have the commit message as the only source of information about why a thing was done, when I always thought that tools that show an overview of the project as a whole (like GitHub/GitLab/Bitbucket/JIRA or alikes) were the standard on most orgs.
|
|
#
?
May 9, 2017 19:36
|
|
|
BabyFur Denny posted:Good news, you don't actually have to upgrade: Thanks! I kept looking at Admin settings, not Repository Settings 
|
|
#
?
May 9, 2017 20:01
|
|
|
HardDiskD posted:On that point, I am now having the impression that people only have the commit message as the only source of information about why a thing was done, when I always thought that tools that show an overview of the project as a whole (like GitHub/GitLab/Bitbucket/JIRA or alikes) were the standard on most orgs. In my experience, they're used inconsistently and poorly and oftentimes provide very little additional information.
|
|
#
?
May 9, 2017 20:05
|
|
|
HardDiskD posted:And if your org works by going all ham in the commit description, like the unix (or git?) greybeards that have to e-mail diff patches around instead of relying on the tools that the community uses at large, then do it.
|
|
#
?
May 9, 2017 20:30
|
|
|
I really hate this thing, and I feel like it must be a well known stupid thing with terminology around it, but I'm not sure what it's called or the best way to describe it. I'm part of Org at BigCompany, and the high-ups in Org issue some edict like 'all products in Org must now X'. X is not fully described, and now there are dozens of teams all attempting to X, and all of them interpret X slightly differently and implement things differently and it's a waste of everyone's goddamn time. Like, if you're gonna issue these edicts, loving understand what you're asking for first and maybe even put together a team specifically for providing an implementation of X for all products in Org.
|
|
#
?
May 9, 2017 22:50
|
|
|
Illusive gently caress Man posted:I really hate this thing, and I feel like it must be a well known stupid thing with terminology around it, but I'm not sure what it's called or the best way to describe it. Allow me to solve for X: 'Agile' ?
|
|
#
?
May 9, 2017 23:13
|
|
|
Cuntpunch posted:Allow me to solve for X: Nah, it's not process/planning stuff, or at least I don't have to deal with those. It's more like 'all products must have feature X', or 'all products must integrate with service X'
|
|
#
?
May 9, 2017 23:33
|
|
|
Illusive gently caress Man posted:Nah, it's not process/planning stuff, or at least I don't have to deal with those. It's more like 'all products must have feature X', or 'all products must integrate with service X' Amazon-style microservices? New social network?
|
|
#
?
May 10, 2017 15:13
|
|
|
Illusive gently caress Man posted:I really hate this thing, and I feel like it must be a well known stupid thing with terminology around it, but I'm not sure what it's called or the best way to describe it. Poor leadership?
|
|
#
?
May 10, 2017 16:35
|
|
|
New Yorp New Yorp posted:Poor leadership? Bad leadership.
|
|
#
?
May 10, 2017 19:50
|
|
|
Blinkz0rz posted:That makes commit messages entirely unhelpful, though. I'm not advocating that you leave the random "WIP" messages, but if a PR is only one commit and it touches a bunch of different things how in the world do you know if the work is appropriate? if a pr needs multiple commits to be clear, it should probably be multiple prs as well? it's pretty common for my team members to make 3-4 simultaneous prs for a feature that may rely on one another
|
|
#
?
May 11, 2017 05:50
|
|
|
the talent deficit posted:if a pr needs multiple commits to be clear, it should probably be multiple prs as well? it's pretty common for my team members to make 3-4 simultaneous prs for a feature that may rely on one another That seems odd to me. By rely on each other are they downstream of a common branch or are all from master? If I ignore or a, b, and d but approve c for merging will it still compile? And will a and b still be valid prs?
|
|
#
?
May 11, 2017 06:00
|
|
|
Hughlander posted:That seems odd to me. By rely on each other are they downstream of a common branch or are all from master? If I ignore or a, b, and d but approve c for merging will it still compile? And will a and b still be valid prs? usually it works like A is an independent commit that adds some new functionality that B and C both rely on, and D is a commit that ties them all together and finishes the feature. B and C will be based off of A (so the prs for B and C include A) and D is based off of a temp branch that has A, B and C already integrated. you can merge A by itself, B (with A implicitly), C (with A implicitly) or D (with A, B and C implicitly). generally the prs are done off of the branch they were actually branched from so you get discrete diffs but they're then merged to master upon approval
|
|
#
?
May 11, 2017 06:13
|
|
|
Question that came up at work: If your single page app is the only consumer of your REST API, and you have end to end feature tests, do you bother to integration test the API itself? It seems redundant to us but maybe there's some benefit?
|
|
#
?
May 11, 2017 12:59
|
|
|
smackfu posted:Question that came up at work: If your single page app is the only consumer of your REST API, and you have end to end feature tests, do you bother to integration test the API itself? It seems redundant to us but maybe there's some benefit? Integration tests are way faster and will more easily pinpoint the source of the error.
|
|
#
?
May 11, 2017 13:02
|
|
|
Define integration test in this instance. I've seen people get very confused over testing terms, and you don't wanna run into any kind of X-Y problem because of that.
|
|
#
?
May 11, 2017 13:46
|
|
|
smackfu posted:Question that came up at work: If your single page app is the only consumer of your REST API, and you have end to end feature tests, do you bother to integration test the API itself? It seems redundant to us but maybe there's some benefit? We had exactly this and what we did was build stubbed backend system tests because they were so fast and then used the SOAP stubs for the services and Selenium for the SPA to test the combined frontend-backend application. Reason was scalability, so that we could plugin another SPA if we wanted to and keep a good view on the continuous quality of the backend application so frontend bugs could be resolved quickly in the correct place and prevent regression on the existing functionality. Did that help?
|
|
#
?
May 11, 2017 14:41
|
|
|
 THIS NEEDS TO GET DONE NOWMe: OK tell me what you need done FIX THIS THINGMe: OK it's done, you needed one more thing, what was that? *crickets*
|
|
#
?
May 11, 2017 21:26
|
|
|
Ghost of Reagan Past posted:
Headless Chicken-Driven Development.
|
|
#
?
May 12, 2017 00:41
|
|
|
Pollyanna posted:Define integration test in this instance. I've seen people get very confused over testing terms, and you don't wanna run into any kind of X-Y problem because of that. That's fair. A test that would require us to spin up a database instance and a real web server. And the thing we would test would be whether the API endpoints return the correct results.
|
|
#
?
May 12, 2017 02:59
|
|
|
smackfu posted:That's fair. A test that would require us to spin up a database instance and a real web server. You shouldn't have to involve your front-end app if you're specifically testing the behavior of the API. The integration tests will be faster if you just write tests for your REST endpoints or whatever. That said, if you're also testing the SPA's behavior then the end-to-end feature testing makes a little more sense even though it's probably slow as hell. It's ultimately up to you and your team to figure out what's most comfortable and easier to pull off. You'll have to make decisions and judgment calls on how much effort should be invested where. If it ain't broke, might as well be too lazy to fix it.
|
|
#
?
May 12, 2017 04:12
|
|
|
Pollyanna posted:You shouldn't have to involve your front-end app if you're specifically testing the behavior of the API. The integration tests will be faster if you just write tests for your REST endpoints or whatever. Ugh, so much of the automated testing of the API functionality at my last job worked by firing off carefully constructed HTTP requests. And yes, they took forever. So somebody multi-threaded them, which made people's dev machines grind to a halt on the rare occasions anybody ran the test locally because of the dozens of threads thrashing around. When I finally implemented some API functionality, I tested it by actually calling the drat functions directly. I wonder what the automated testing looks like now, after the other guy who was doing them left voluntarily and I was laid off.
|
|
#
?
May 12, 2017 04:45
|
|
|
CPColin posted:I wonder what the automated testing looks like now, after the other guy who was doing them left voluntarily and I was laid off.
|
|
#
?
May 12, 2017 05:48
|
|
|
We released a UI update to one of the sections of our product last week. This update has been in development for at least a month and a half and has been shown in review at least three times and has received praise and some feedback each time. Today one of our pro services people saw it and said it looks retarded. Someone from pro services has been to each review where this update was shown and they had ample opportunity to comment on the way it looked and how the current version was used. They did not. We took issue with this. Edit: What really burns me is that there is something we can do to make the situation better: we can get these features onto a UAT environment earlier, because it's hard to get a feel for new poo poo when it's just being shown to you. But what's going to end up happening, and I KNOW it will happen, is that we'll spend the extra effort to get it onto the UAT environment and tell the rest of the company exactly what's updated and how they should get to it, and they just won't. They're not going to bother. At least if we do that we can tell them to pound sand if they bitch after it goes live. In any case they were way over the line with their comments today. Che Delilas fucked around with this message at 06:50 on May 12, 2017 |
|
#
?
May 12, 2017 06:46
|
|
|
Ghost of Reagan Past posted:
FTFY
|
|
#
?
May 12, 2017 15:54
|
|
|
I think I'm burning out pretty hard and pulling down the rest of my team. Not entirely sure how to broach the subject with my boss. Whiny rant. Venting. Please ignore. I've been doing the AI/ML work for our project, particularly as it relates to search and document clustering. My stuff is way behind, but I get to the office and sit down with a task I've broken into what I think is a tiny digestible piece like, "write document models to preproduction," and can scarcely muster the energy to do that. Partially, I think it has to do with the knowledge that it's going to take a few days to write all the documents to the database, there's a 50/50 chance it will crash in the middle and I'll have to start over, and ideally I'll catch poo poo from our DBA who insists I need to find a smarter way to insert records. He's right, of course -- I'm not a good developer. But loving hell, it's a multi-million dollar company and we have maybe 60 users of the database. Is it really unreasonable to insert a few million rows on a Friday afternoon? I feel like everything I do takes longer than it should by a factor of 10. I'm about 99% sure it's my fault, too, but when it comes to actionable solutions, I'm coming up blank and the continuous pressure of underperforming has crushed my will. I don't even feel like working on my personal projects any more. This has been a test of the emergency whiny rant system.
|
|
#
?
May 12, 2017 17:27
|
|
|
Jo posted:I think I'm burning out pretty hard and pulling down the rest of my team. Not entirely sure how to broach the subject with my boss. Well, if the DB is right now a major hurdle, I see two options to resolving it. One, spend some time with the DBA to find that better way to insert records. He should help you with this, that is what DBAs are SUPPOSED to do, not just bitch (which is what they usually do). Two, if the DBA is not helpful, or admits there IS no better solution, maybe you need some dedicated hardware. Make a case how you will be X% more productive if the DB isn't crashing all the time.
|
|
#
?
May 12, 2017 17:37
|
|
|
|
| # ? May 14, 2024 12:22 |
|
|
Jo posted:I need to find a smarter way to insert records. I'm not sure you were looking for technical advice or not but if yes : Are you using SQL Bulk Insert and SQL Merge or the equivalent of your RDBMS ? Inserting a few million rows using individual insert statement is a big no-no.
|
|
#
?
May 12, 2017 17:39
|
|