VBA Rubberducking

Duga

00:00

RELOAD!

[rubberduck-vba/Rubberduck] 14 commits. 2 closed issues. 1 issue comment. 5192 additions. 6010 deletions.

IvenBach

Note to self: Practice atomic commits. #QuitCrossingTheStreams

Too easy to be sloppy and merge commits that shouldn't be together.

Vogel612

I got used to using git add -p instead

IvenBach

I couldn't understand what that does.

Comintern

@IvenBach I'm guessing you don't mean "atomic commits" as in "nuke from orbit commits".

IvenBach

No. As in each commit is self containing. Doesn't inadvertently mix in something else.

I still get too many hairs in the batter because I'm not the best at atomic commits.

Vogel612

00:10

geez I'm bad at this

I spent like half an hour on trying to figure out a way to put Func into a generic type specification when I didn't need to in the first place

Jelly

I love generics.

They are so ...

generic lol

Vogel612

hey, I'm down to 4 failed tests

one broke, the rest broke so hard that they weren't even counted

Comintern

Grrrr... The stupid resx designers aren't updating for a merge commit.

Vogel612

'course not

IMO we shouldn't even be versioning them, tho

so there's that

Duga

[rubberduck-vba/Rubberduck] comintern synchronized pull request #4488: Add new inspection for Excel UDFs hidden by cells.

IvenBach

00:33

Home Time.

Jelly

30 mins till home time!

Comintern

AV isn't exactly ripping it up on speed today. Almost 15 minutes now...

Vogel612

the Core build process isn't really fast, yeah

this

@Vogel612 I always wondered about that one

why aren't we excluding it from git?

Vogel612

dunno, actually

this

00:41

@Vogel612 It also doesn't help that it's the bottleneck between the 2 groups of projects

Core can't build without any of 8ish projects, and nothing else can get built until Core is built.

Vogel612

I wasn't originally talking about Rubberduck.Core, but... yes

this

oh my bad

Vogel612

Oh you're fully correct

Duga

[rubberduck-vba/Rubberduck] build for commit 2ed334e4 on unknown branch: AppVeyor build succeeded

this

speaking of building...

Vogel612

00:42

I may have fixed it

Duga

[rubberduck-vba/Rubberduck] comintern removed label PR-Status: Conflicting from pull request #4488: Add new inspection for Excel UDFs hidden by cells.

Vogel612

but it's a bit rather fugly still

this

it being what?

Vogel612

the tests and the renaming thing

this

cool. What's ugly about it?

Vogel612

00:44

multistage mock setup

and mock setup while executing prod code

this

IIRC there's a sequential setup for Moq

need to remind myself why twice, though.

Vogel612

because show has two overloads and at that place we don't know which of the two is called

so we set up both with the same thing

yea... down to 22 failing

this

at least you can abstract that into a function

Vogel612

none inconclusive, though

this

and more importantly, that means that even if the presenter changes, test code shouldn't change.

Vogel612

00:48

@this that's in the PerformExpectedVersusActualRenameTests, so I won't just yet

I took the libery to move a ton of test execution stuff out of the testclass, though

this

cool

Vogel612

just to keep my sanity when scrolling through either of the four intermingled concerns

this

i'll admit it gave me a headache to look at the test class and figure out what we were really testing

Vogel612

ugh. for whatever effing reason VS thinks it must run CodeAnalysis before running tests

and that makes executing a single unit test slow AF

something for another time tho

Duga

[rubberduck-vba/Rubberduck] comintern synchronized pull request #4488: Add new inspection for Excel UDFs hidden by cells.

this

01:00

@Duga hmm I might have messed that up

so to see that i understood - we don't want the designers enumerated in the csproj?

Comintern

No, I'd just mucked around with the project to force them back into sync.

this

because I actually did the same thing when I merged my other PR

thinking we needed to enumerate them

Comintern

Forgot to change it back after I built. I think my merge may have gotten them out of sync.

@this Couldn't hurt, but that proj file had all the localizations that don't actually need to be built.

I think the problem might be when you switch branches it messes up the build because it doesn't pick up the change to the .resx file coming via SC.

Just spit-balling there though. No clue why that breaks it.

this

that sounds consistent with what I saw before

problems usually happens only after switching branches, AFAIHS

Comintern

I should address some of the 591 build warnings.

this

01:09

maybe the answer is here: stackoverflow.com/a/12206011/643342

nah. we need 75 more.

Comintern

I could do 75 more warnings in a heartbeat.

this

then it'll be fully-certified ThunderCode™.

Comintern

@this I don' t think that would be a bad idea at all. At very least we'd all be building the same thing as AV locally.

this

yeah, a bit of sledgehammer but that'll ensure that clean actually cleans.

IDK if that'll fix the designers, though - aren't they in the same directory as the source files, though?

Jelly

@Hosch250 youtube.com/watch?v=zufgsUEeHmc

Comintern

01:16

A bunch of these are just adding sealed to classes with explicit implementations.

Duga

[rubberduck-vba/Rubberduck] build for commit 3643c7ec on unknown branch: AppVeyor build succeeded

this

we can suppress most of the MIDL compiler's warning by passin gin a command line, too

I don't care about not using variant - ours are dual interface so it's OK

Vogel612

TTGTB.

Toodles!

this

night!

Comintern

'night

@this I thought we wanted to pass variants because of the permissive assert thing.

i.e. bypass the marsaller

this

01:25

nah

permissive assert just need to call oleaut functions

if necessary, we can make permissive assert class marshal their stuff as IntPtr

that should keep marshaller's grubby paws off it.

so I'd just suppress both MIDL2400 and MIDL2401 and worry about other warnings more.

Comintern

Down to 391

this

i need to update the deployment so I'll take care of that

Comintern

@this Right, but if RD is passed a Variant and marshals it to a managed object, how do we reversibly get the native Variant back to pass to oleaut?

Granted, there are a bunch we shouldn't care about.

I think the IDisposable interfaces are next...

Duga

[rubberduck-vba/Rubberduck] retailcoder merged pull request #4519: Include property accessor type in the "search results" tab from retailcoder/Rubberduck/bugfix into rubberduck-vba/Rubberduck/next

[rubberduck-vba/Rubberduck] retailcoder pushed commit 6fc894d2 to next: closes #4479

[rubberduck-vba/Rubberduck] retailcoder pushed commit 685204af to next: added property accessor type to tab name

[rubberduck-vba/Rubberduck] web-flow pushed commit 83e94b22 to next

Merge pull request #4519 from retailcoder/bugfix

Include property accessor type in the "search results" tab

[rubberduck-vba/Rubberduck] retailcoder closed issue #4479: "Find all references" missed a couple

Comintern

CA1060 might want to be its own PR too.

Duga

01:31

[rubberduck-vba/Rubberduck] retailcoder moved project card

this

@Comintern we don't need it. Just take it as IntPtr and forward to the oleaut.

Comintern

I was thinking of marshaling to a Variant struct, but that works too.

this

and if we really do need the object, Marshal.VariantToObject (or however it's spelt) will do it for us

docs.microsoft.com/en-us/dotnet/api/…

GetObjectForNativeVariant(IntPtr)

KISS

Yeah, actually CA1060 is one of the item on Vogel's tech-debt list.

Comintern

Yeah, that will work.

Duga

[rubberduck-vba/Rubberduck] retailcoder merged pull request #4488: Add new inspection for Excel UDFs hidden by cells. from comintern/Rubberduck/validref into rubberduck-vba/Rubberduck/next

[rubberduck-vba/Rubberduck] comintern pushed commit 0b93e503 to next: Add new inspection for Excel UDFs hidden by cells.

[rubberduck-vba/Rubberduck] comintern pushed commit 62dfc351 to next: Add inspection defaults to Settings.settings

[rubberduck-vba/Rubberduck] comintern pushed commit 2ed334e4 to next: merge next

[rubberduck-vba/Rubberduck] comintern pushed commit 3643c7ec to next: Roll back change forcing resx designers back into sync

[rubberduck-vba/Rubberduck] web-flow pushed commit fb0f9b7e to next

Merge pull request #4488 from comintern/validref

Add new inspection for Excel UDFs hidden by cells.

[rubberduck-vba/Rubberduck] rubberduck-vba-releasebot published prerelease Prerelease-v2.2.0.4168

[rubberduck-vba/Rubberduck] rubberduck-vba-releasebot created tag Prerelease-v2.2.0.4168

[rubberduck-vba/Rubberduck] build for commit 83e94b22 on next: AppVeyor build succeeded

[rubberduck-vba/Rubberduck] bclothier synchronized pull request #4491: Clean COM wrappers leak: Round 2

Comintern

02:03

Hmmm... So ICommandBar implements ISafeComWrapper, which is also implemented via ISafeComWrapper<T> by SafeComWrapper<T>, which CommandBar inherits from. WTH does that mean that CommandBar should explicitly override the base class dispose method?

I can't see any conceivable situation where the base Dispose wouldn't be called.

Duga

[rubberduck-vba/Rubberduck] rubberduck-vba-releasebot published prerelease Prerelease-v2.2.0.4169

[rubberduck-vba/Rubberduck] rubberduck-vba-releasebot created tag Prerelease-v2.2.0.4169

[rubberduck-vba/Rubberduck] build for commit fb0f9b7e on next: AppVeyor build succeeded

this

@Comintern doesn't best practices (at least as sez by Microsoft) decree that all derived class ought to override the Dispose(bool Disposing) method?

Comintern

Yes, but I'm not sure that's relevant here. The point in doing that is that GC.SuppressFinalize(this); is called, but in this case it's called in the base class.

this

hmm. are finalizers the same for all hierarchy?

or does each node get their own finalizer?

Comintern

Hmmm... Not sure.

this

02:14

would hvae to review the dispose pattern to be sure

they can't have written warning just for funnies.

Comintern

The warning text seems to indicate it violates this rule: "IDisposable is reimplemented in the class."

> Warning CA1063 Microsoft.Design : Remove IDisposable from the list of interfaces implemented by 'VBProjects' and override the base class Dispose implementation instead.

It seems to want IDisposable removed from the IVBProjects interface, which makes no sense at all.

Ahhh... Finalizers are not inherited.

this

makes sense.

ctors aren't. why would dtors be?

Mathieu Guindon

@this base ctor always runs first though, given a base class

Comintern

Wouldn't that imply that the dtors run in reverse order?

this

True but that's not true inheritance, is it?

normally when there's no parameterless ctor, we must provide our implementation of the ctors even if it's an empty body

Mathieu Guindon

02:21

the base default ctor always runs...

this

for all I know, dtors might have its own rule apart from the ctors.

Mathieu Guindon

Comintern

I wonder if it just wants this:

    private bool _disposed;
    protected override void Dispose(bool disposing)
    {
        if (_disposed)
        {
            return;
        }

        base.Dispose(disposing);
        _disposed = true;
    }

That seems silly if there isn't any work in the derived class though.

this

do you even need all that, though? just calling base.Dispose(disposing) ought to suffice.

Comintern

...and that's exactly what it wants

Mathieu Guindon

02:24

@this then why override at all?

this

the base.Dispose should already have the _disposed to guard against redundant calls.

Comintern

Probably not.

this

feh. I know nothing.

Comintern

LOL. If you remove everything but this...

    protected override void Dispose(bool disposing)
    {
        base.Dispose(disposing);
    }

... it gives a new warning for redundant override.

It does clear the CA1063 though.

Duga

[rubberduck-vba/Rubberduck] build for commit 599e852d on unknown branch: AppVeyor build succeeded

Mathieu Guindon

02:25

ha! see? :p

Comintern

Where do I complain?

Mathieu Guindon

there's gotta be a better way to clear it?

this

hmm i'm a bit foggy but could it be because we have IDisposable used more than one time?

Comintern

pragma ignore

this

like defined on the interface then again on the class?

Comintern

02:28

Although that's one line of code, just like protected override void Dispose(bool disposing) => base.Dispose(disposing);

Mathieu Guindon

CA1063 lists "Dispose() is overridden." under "causes"

IOW "Implement IDisposable correctly: don't"

Comintern

I thought at first it was because it inherits the implementation from 2 different interface paths.

Mathieu Guindon

seal it

Comintern

sealed makes no difference.

this

hmm can you seal the base's dispose method?

the article seems to imply you can

Comintern

02:32

You mean remove the virtual?

this

yeah, i guess

Mathieu Guindon

> If you create an unsealed type that declares and implements the IDisposable interface, you must define Dispose(bool) and call it.

this

I was looking more at this:

> The Dispose() method is not public, sealed, or named Dispose.

Mathieu Guindon

"The Dispose() method is not ... named Dispose". yup. 500000K unit tests.

this

TBH I can't imagine how they could implement the IDisposable interface and not have a Dispose method named..... Dispose.

Mathieu Guindon

02:34

> Rename your dispose method to Dispose and make sure that it's declared as public and sealed.

Comintern

WTF?

MyDispose()?

this

#Unpossible

Mathieu Guindon

basically I read it as the IDisposable.Dispose shouldn't be overridden, and the base class must supply a templated method for the inherited behavior.

Comintern

Right - Dispose(bool) should be protected and have an override.

It just seems pedantic if it only called the base.

this

but we're talking about Dispose() - you said it's virtual?

Comintern

02:37

No, the base class follows the correct pattern.

this

ok.

Comintern

protected virtual void Dispose(bool disposing)

^ That's what it's whinging about apparently.

this

there's this, though....

> If you create an unsealed type that declares and implements the IDisposable interface, you must define Dispose(bool) and call it. For more information, see Clean up unmanaged resources (.NET guide) and Dispose pattern.

Mathieu Guindon

is it called?

Comintern

It wants all the derived classes to override that.

Mathieu Guindon

02:38

ah

Comintern

The Dispose() method is public and not virtual.

this

which is kind of lame since we have nothing extra to do.

Comintern

IKR?

Mathieu Guindon

@this IMO the docs are describing the templated method there

Comintern

I guess if nothing else it means that if we ever do need to add something it will be clear the base class needs to be called.

Which is completely obvious to begin with...

this

02:39

hmm. apparently you can seal methods.

Mathieu Guindon

^ yeah

this

memo to myself- re-study why virtual even exists if a method can be sealed

Mathieu Guindon

because methods aren't virtual by default in C#. they are in Java though ;-)

Comintern

What c# version. I can't figure out where it wants the keyword.

Mathieu Guindon

public sealed void foo

Comintern

02:42

Isn't everything virtual in Java?

Ahhh... you can only seal an override.

public sealed string ToString()

Mathieu Guindon

I like this:

> When to suppress warnings
Do not suppress a warning from this rule.

Comintern

I already have one :-D

We shouldn't ever dispose the dockable hosts

this

can they not be IDisposable?

Duga

[rubberduck-vba/Rubberduck] bclothier removed label PR-Status: Conflicting from pull request #4491: Clean COM wrappers leak: Round 2

Comintern

@this The inherit from an IDisposable base class.

this

02:53

of course. would have been too easy.

they do have finalizer, though?

Comintern

Yep.

I think.

It might just get cleaned up by the OLE detach call - I'll have to check.

this

i'm pretty sure hte way it's working now is fine.

we shouldn't be throwing any more exceptions on shutdown (we were until Wayne worked around that bug in the interop code)

sigh. VS, VS, VS. I say to clean a project. Surely you know the difference beetween a project and a solution

Duga

[rubberduck-vba/Rubberduck] web-flow pushed commit c08afbad to next: Update README.md

this

@Duga tbh i liked the prior icon - seemed more fitting for the masthead

Comintern

WTH is going on here?

        public void Dispose() => ((IDisposable)this).Dispose();

        private bool _isDisposed;
        void IDisposable.Dispose()
        {
            if (_isDisposed) return;
            _isDisposed = true;

            foreach (var element in _list)
            {
                element.Dispose();
            }
        }

Why the 2 overloads?

Mathieu Guindon

03:06

one with an explicit interface implementation

this

not sure that's a case to need a explicit implementation, though.

I'd try and get rid of it. See if it makes VS cry.

Mathieu Guindon

IKR

Comintern

The one dispose pattern fired 5 different warnings.

Duga

[rubberduck-vba/Rubberduck] bclothier created pull request #4525: Deployment bug fixes to merge bclothier/Rubberduck/FixDeployment into rubberduck-vba/Rubberduck/next

> Add path checks & directory creation for missing folders.
Add pragmas to generated IDL files to ignore MIDL 2400 and 2401 warnings.

[rubberduck-vba/Rubberduck] bclothier added label PR-Status: Review Requested to pull request #4525: Deployment bug fixes

this

@Comintern you might need to write to the mothership

Duga

03:17

[rubberduck-vba/Rubberduck] bclothier assigned bclothier to pull request #4225: Adds Add Predeclared Class Module Command

[rubberduck-vba/Rubberduck] web-flow pushed commit 7681361d to next: Update README.md

Mathieu Guindon

@this any idea how to get rid of the borders?

eh, you're right.. it needs an actual banner

this

They would say to not use a table

i think you need an inline style to make tvable border transparent

with div, borders are already transparent but you would still have to align them to get grid like layout

Duga

[rubberduck-vba/Rubberduck] build for commit bbfce561 on unknown branch: AppVeyor build succeeded

[rubberduck-vba/Rubberduck] web-flow pushed commit f9881bce to next: Update README.md

Mathieu Guindon

03:35

#ProblemSolved

Comintern

304 down, 287 to go.

this

@MathieuGuindon heh clever

Mathieu Guindon

note to self: avoid git pull upstream next with a large diff and VS opened.

@this #4491 is good to go right?

IvenBach

04:29

Where did all these conflicts or failed tests come from?

Comintern

@IvenBach Which conflicts and/or failed tests?

The build warnings?

this

@MathieuGuindon afaik, yes. Unless. Others has a contrary opinion

IvenBach

It seems like you and Vogel both are conquering failed tests.

I've only been somewhat following chat.

this

Vogel is working on my half assed pr for refactoring dialog refactor

Comintern

I'm working on code analysis warnings. Those are basically tech debt.

this

04:32

this one has 500 broken tests give or take 250 Yay me.

IvenBach

That's what we get for allowing noobs to work on the RD code base.

Comintern

Nah, they're everyone's.

this

^

Comintern

The Dispose stuff is just laziness for the most part.

IvenBach

Is it just part of the learning process everyone's gone through with RD?

Comintern

04:34

No, it's trying to remember the overly pedantic pattern that MS looks for with Dispose.

this

It is always a learning process

IvenBach

I'll have to understand it after I see it. Isn't it just Foo.Dispose()?

this

Read this thoroughly docs.microsoft.com/en-us/dotnet/standard/garbage-collection/…

Hmm.. @Comintern. Any reasons why we aren't using SafeHandle. Apparently MS recommends it over IDisposable.

Comintern

SafeHandle is a monumental PITA.

Not that our current setup isn't...

this

Oh? GTK.

Comintern

04:45

There are some legit issues in the CA warnings.

CA1901 picked up a couple bad signatures.

IvenBach

05:30

@Vogel612 ^ Referring back to the German meal that was made for me. That's one of the items I snuck a photo of. Do you know that brand/is it any good?

Duga

05:50

[rubberduck-vba/Rubberduck] comintern created pull request #4526: Cull build warnings to merge comintern/Rubberduck/next into rubberduck-vba/Rubberduck/next

> Most of this is implementing the proscribed Dispose pattern, although there are a couple of legit warnings that were fixed and a couple non-legit ones that were suppressed.

[rubberduck-vba/Rubberduck] comintern added label PR-Status: Review Requested to pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] build for commit ef02b69d on unknown branch: AppVeyor build failed

BUILD FAILURE!

Vogel612

10:06

@IvenBach dunno, that's Bavarian. Never seen it before

Duga

13:50

[rubberduck-vba/Rubberduck] bclothier commented on Rubberduck.Core/UI/CustomComWrappers/IOleWrapper.cs of pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] bclothier commented on Rubberduck.Core/UI/ISaveFileDialog.cs of pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] bclothier commented on Rubberduck.VBEEditor/ComManagement/TypeLibs/TypeLibs.cs of pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] bclothier commented on Rubberduck.Core/UI/DockableToolwindowPresenter.cs of pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] bclothier commented on Rubberduck.Core/UI/IOpenFileDialog.cs of pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] bclothier commented on RubberduckTests/Mocks/Windows.cs of pull request #4526: Cull build warnings

Duga

15:30

[rubberduck-vba/Rubberduck] rkapka removed label PR-Status: Review Requested from pull request #4488: Add new inspection for Excel UDFs hidden by cells.

Duga

16:05

[rubberduck-vba/Rubberduck] retailcoder merged pull request #4525: Deployment bug fixes from bclothier/Rubberduck/FixDeployment into rubberduck-vba/Rubberduck/next

[rubberduck-vba/Rubberduck] bclothier pushed commit bbfce561 to next

Add path checks & directory creation for missing folders.
Add pragmas to generated IDL files to ignore MIDL 2400 and 2401 warnings.

[rubberduck-vba/Rubberduck] web-flow pushed commit 862e7221 to next

Merge pull request #4525 from bclothier/FixDeployment

Deployment bug fixes

[rubberduck-vba/Rubberduck] comintern commented on Rubberduck.Core/UI/DockableToolwindowPresenter.cs of pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] comintern synchronized pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] comintern commented on Rubberduck.Core/UI/CustomComWrappers/IOleWrapper.cs of pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] comintern commented on Rubberduck.Core/UI/IOpenFileDialog.cs of pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] comintern commented on Rubberduck.VBEEditor/ComManagement/TypeLibs/TypeLibs.cs of pull request #4526: Cull build warnings

[rubberduck-vba/Rubberduck] comintern commented on RubberduckTests/Mocks/Windows.cs of pull request #4526: Cull build warnings

this

@Duga isn't this now violating the best practice RE: dispose? Or was there actually a problem with using the Dispose(false) in the finalizer?

Duga

16:25

[rubberduck-vba/Rubberduck] rubberduck-vba-releasebot published prerelease Prerelease-v2.2.0.4173

Comintern

@this The finalizer should only need to call dispose if there are unmanaged resources.

Duga

[rubberduck-vba/Rubberduck] rubberduck-vba-releasebot created tag Prerelease-v2.2.0.4173

[rubberduck-vba/Rubberduck] build for commit 862e7221 on next: AppVeyor build succeeded

this

@Comintern I see. I thought it had unmanaged resources but I see that's not the case.

However, there's a UserControl there, too. Not sure if we're supposed to dispose it (IIR, it's disposable but I might be wrong)

@ThunderFrame hope you're doing well! I hope you don't mind me working on your awesome PR. I'm cleaning it up and will make it compatible with VB6; in middle of putting in the necessary abstractions.

Comintern

Is the UserControl disposed by the IOleWrapper?

this

ah i see - i thought that was WPF. That's actually WinForms.

Comintern

16:34

Oh wait, no - that should be cleaned up by the SubclassingWindow via the message pump.

this

there's this comment

> // WARNING: Disposal of _userControl / _cachedClientSite should be handled in IOleObject::Close(), not here, see top comments

so the code is just fine as it is (not disposing the usercontrol)

given that both WPF and WinForms has UserControl it might do us good to be clear in labeling those....

Comintern

I thought so - those have crap that needs cleanup all over the place.

this

or at least a comment to link to that comment above. It wasn't evident from reading the DockablePresenter alone who owned the usercontrol.

(and we have many code like that... :-\ )

Comintern

^

this

I hope they'll get IDisposable analyzers updated soon, so we can use it, and encourage annotating. That'd go long way toward clarity when reviewing the code, especially those "here-be-dragons" sections.

M.Doerner

16:41

@Duga I would prefer interfaces never to be in the same file as an implementation.

this

Unfortunately, we have a number of files like that.

I think it was mostly out of laziness since there was only one implementation.

I would agree that it'd be ~~necessary~~ mandatory to split once there's more than one implementation.

but am ambivalent about one file housing both single interface + single implementation.

Duga

[rubberduck-vba/Rubberduck] build for commit 5c4b51c2 on unknown branch: AppVeyor build succeeded

Comintern

@M.Doerner Generally I agree. I'm not a big fan of multiple classes per file either.

this

@Comintern are you planning on addressing the other remaining warnings (besides the ones about P/Invoke) with the PR?

Comintern

I was planning on working my way through the list. Nothing that says they need to be in the same PR though.

this

16:50

cool

bbl

Duga

17:09

[rubberduck-vba/Rubberduck] Wainers starred us

3

Inarion

17:23

How contributing works - is this accurate/applicable for RD?

And are you guys using the GitHub Extension for VS or some other means to manage the GitHub related things? (I've seen @Vogel612 mention command line, though I'm not sure in what context that was.)

Duga

[rubberduck-vba/Rubberduck] Inarion forked us into Inarion/Rubberduck

2

Comintern

@Inarion Both. IIR the GH extension will also install the CI tools.

Inarion

17:40

@Comintern I see. Thanks!

How much space does one branch consume? Not much, I guess? (In the regime of MBs?)

Vogel612

more like bytes

Mathieu Guindon

@Inarion this wiki page means to have everything you need to know for the initial setup: github.com/rubberduck-vba/Rubberduck/wiki/Contributing

Vogel612

possibly kilobytes, but probably not even that

Inarion

@Vogel612 that's good. Needed to remove a couple of programs to make room for VS.

@MathieuGuindon I've got that page open - but my current steps seem to precede what's written there. :)

Mathieu Guindon

@Inarion with SVN, each branch is a folder, so branches cost quite a lot. With Git, the working directory is whatever the current branch is, so git branches are very lightweight - ideally when you start a work item you make a new branch just for it, and then PR off that branch. that way your "next" branch can be kept in sync with the central repo (aka "upstream"), and it's easy to make new, parallel branches

That said the RD repo weights something close to 10GB

Inarion

17:48

@MathieuGuindon You're saying upstream pulls directly from the individual branches, instead of my origin?

Or, rather: It should do that.

This then results in more atomic PRs to the central repo, right? I guess that makes tracking issues back to specific changes easier?

Mathieu Guindon

no, upstream isn't a branch, it's a remote - it's https://github.com/rubberduck-vba/Rubberduck.git

origin is also a remote: it's your fork

when you clone your fork, you get a local, working copy. You make changes, commit them; then you push the commits to your fork, and then make a PR from your fork to the central repo

Vogel612

@MathieuGuindon huh? It's just 1GB on my machine

what have you done?

Inarion

@MathieuGuindon Ok, I think I got that. I'm probably a bit inaccurate on the vocabulary, but that's seems easy enough. :)

Mathieu Guindon

@Vogel612 wah, I'll check again...

Inarion

Alright, now I've got a Rubberduck project open in VS. Will figure out how to proceed after cooking/having dinner. Later!

Vogel612