Independent Dev Testimonials

Just skimmed through all the files on the github. The code looks clean and well thought out, not everything is as self-evident however if you just look at what functions are being referenced it makes a lot more sense. Some commenting might help for outsiders but also add lots of additional space into the files and if you frequently work with the code it might be more of an obstruction than helping out to re-read the same comments scrolling through it.

I don’t know Rust but I do see it uses types everywhere which only makes code less error prone and act better in according what you expect it to do. Hope one day I can code like this too and it made me interested to learn the Rust coding language!

Some parts I saw comment with allow(dead_code), does this mean it is no longer used or that it will be removed?

Bottom line, this looks like quality code to me and shows the devs work with a complex concepts while maintaining separation of concerns. :clap: :+1:

@dirvine Is there a visual tool that can show relations in code? Like file x.rs has function a,b,c that y.rs uses etc. Would be very easy to see for someone not familiar with the code how everything relates and depends on each other.

8 Likes

Re tooling, I’ve reluctancy succumbed to VSCode which has excellent support for Rust and every other language I use (and probably better than commercial dedicated products).

This includes ways to find what uses particular functions, shortcuts to visit declaration, implementation etc. There’s a free community add-on for just about anything a programmer can think of, because so many did and then created it.

Some stunning stuff, including whole applications ported to run inside VSCode.

How did I miss this?

4 Likes

I use IntelliJ Rust as I have a license for my Java work. It lets you drill into function/method calls, including those in external dependencies where possible. This really helps to build up a visualisation of the flow in my head.

IntelliJ Rust in general seems decent, but I’ve not used it in anger yet. It nicely colour codes syntax and makes it easy to navigate though.

2 Likes

In VS Code you can also right click a symbol and click “browse references” which gives you a list of callers, etc. Not quite what you’re looking for, but you can hit Ctrl+t and tag on it from there and walk the rabbit hole yourself. Not quite as good, but still a feature I use quite often myself.

Thoughts on the Current Codebase

Now on to the “state of the code”, as told according to the opinion of one highly-fallible engineer. Please be gentle :slightly_smiling_face: I’m a little late to the party, but incoming brain dump.

TLDR;

The term “beta” is used and abused these days in all the wrong ways. When David says “beta”, I would say its smart to interpret it in the most literal sense possible. I take that to be that the major architectural pieces and algorithms are in place. It’s functional, but everything else is up for grabs still.

I do not expect that “beta” implies “nearly finished,”. Interpret it as closer to “minimum number of big features implemented” (not the same as “feature-complete”). I get the impression this is probably the “end of the beginning” or perhaps the “beginning of the middle”.

Accessibility

I’m going to comment on the current state of the code base through the lens of contribution accessibility. I think this reflects the level of maturity of a codebase well, since more mature code can devote more resources, all else equal, to finetuning, tweaking, and supporting community development efforts. I’ll break it up by crate for the crates I know about to some extent.

  • sn_api - Accessibility : 8/10
    • In my opinion the best entry-point into the codebase. Very well-documented (as it should be, since its front-facing for users) and architecture is simple/easy to figure out at a glance. As a result, it’s simple to find ways to improve the code or fix bugs.
  • sn_node : Accessibility: 3.5/10
    • Not for the faint-of-heart. You really need to get your hands dirty to figure out what’s going on. It generally feels like a construction zone still, and there are lots of TODO comments to be seen.
    • You better be up-to-date on the current state of the net algorithms to some degree before looking at this. The architecture is not tightly coupled, which is nice and flexible, but also makes it tedious at times to figure out call-chains unless you have an idea of where it might be going next already from prior knowledge.
      • This is especially if you aren’t familiar with this kind of heavy indirection in code. I imagine it’s even harder to grok if you’re used to traditional object-oriented programming in the context of something like web-dev.
    • Related to the above, the architecture isn’t the most straightforward, and it’s been changing a lot until recently (4500 line patch to remove some message wrapping logic was the last big one, which occurred only last week). According to David, this should calm down now, but historically it’s proved to be a challenge. That said, I still see lots of room for improvement in architecture, so I’m not sure how much that’ll hold true beyond the next test net.
      • Quick, rather technical example: mutably borrowing the whole Node for every node operation, regardless of whether it needs the whole thing or not, defeats the performance gains entirely of our async/await calls. This can’t be fixed trivially with an Arc<Mutex> because one message generates another message so you need to borrow the whole thing “just in case”.
    • Documentation is spotty, as mentioned, which doesn’t ease things. That’s the least of it though, because of the general effort required to contribute to this (e.g. understanding network algorithms & architecture), you can probably follow a few call chains to piece together any missing docs.
  • sn_routing & qp2p: Accessibility: 6/10
    • These are actually rather straightforward, but I’ve little experience contributing to them actively. The code looks clean enough and isn’t too hard to figure out. It suffers heaviest I think from the “how to contribute” problem outlined below.
  • I don’t know about ffi or some of the other stuff. I have a passing knowledge of BRB after browsing a bit, but not enough to comment on thoroughly either, but it seems to be among the most experimental code still.

Biggest Barrier to Contributors: HOW can I help?

Even if you’re comfortable enough to understand existing code and work with it, knowing HOW to contribute is still difficult. I find it’s difficult to know at a glance, without a lot of effort, to figure out what what needs to be done, which tasks are currently being worked on already, and which are still up for grabs for somebody to step in and help.

Example: There’s no great place to figure out what bugs need squashing, what areas of code need more tests, etc. This speaks to the fact that devs still seem concerned heavily with making things work more so than making sure there are breadcrumbs to follow.

The GitHub issues don’t help a ton, and are a hit or miss. Sometimes stale issues stick around for a while. Sometimes the issues are sort of non-specific and just general reminders. Sometimes code efforts aren’t tracked by an issue at all until its time to PR, which makes it easy to accidentally waste time on something that will be superseded soon by a patch (which is why it’s a good idea to run things first on the dev forum right now I think).

The easiest contributions are new features (e.g. a new cli command or something), since there’s fewer conflicts with existing code, and they exist outside of ongoing efforts. That said, new features are not always the most immediately useful, and make it hard for the community to help meaningfully in terms of things that will be relevant in the semi-near-future (e.g. tests, bugfixes, and refactors). Any bugfix/refactor/etc. I’ve personally submitted is the result of stumbling across it in the code base while doing something else entirely. It’s not for a lack of things to do, but a lack of the bandwidth needed to painstakingly document and triage them.

Conclusion

The code base is settling down, but it’s still not easy to contribute. Things have historically moved fast, and, while it’s getting slower, this indicates to me that I can hardly call the code “mature” either. There are a lot of unimplemented things still, even if the last of the bare-minimum functional pieces is almost in (referring to rewards).

I hope this doesn’t come off the wrong way. I’m just reflecting a bit on some things I’ve experienced or seen others experience along the way, and what that might indicate about the current state of “done-ness”.

18 Likes

IMO: PR author politely calls out bikeshedding as they saw it. Folk agree so PR is tested + reviewed, merged, and the rest dealt with down the line.


@anon57419684 you appeared to be engaged in that discussion you’ve now decided was bikeshedding…

There was an attempt there in that thread there was to get more input from more folk to start opening up such things to more contributions/reviewers etc. (There has been a sizeable uptick in contributions of late, which is great, but it’s also hard to stay up to speed with it all; so such things are going to be a necessity. Which I think is a good thing in terms of making folk feel more ownership of code as things start to stabilise).

If anyone feels a PR is getting bikesheddy, then the best route forward (I think… open to other ideas about how to address this) is to bring that up in the discussion. Bowing out with, “i dont have time” is fine. But it doesn’t do anything to highlight or address the issue. I was not aware you considered that to be bikeshedding until just now, eg. I had imagined you may come back to the issue in a week or so when you had more time.

I’m not sure anyone else there saw it as such either… (though I can see where you’re coming from; anyone on that thread can happily correct me if I’m wrong).

Perhaps there are better ways to address the bikeshed concept? Do folk have experience with other OS projects that deal with that in some fashion?

11 Likes

Just wanted to share that I agree with this statement. Sorry you felt that way @anon57419684, I thought it was a pretty engaging discussion at the time. If it happens again, maybe just call it out if you’re comfortable with that? This community is pretty tight and I’m sure people would understand.

To be fair, I think it’s easy to come off as “bike-sheddy” inadvertently. Especially in engineering and especially when people care about the product. In this case though, I think it’s less about punting and more that the devs here all care deeply about the work and want to come to the best version of the code that’s possible. Having experienced this personally, in a large corporation where nobody can be asked to give a damn, being punted looks a little different than this IMO.

Sometimes it’s easy to get a little carried-away, sometimes code just doesn’t make it to master for one reason or another, and sometimes people come off as pedantic. But I don’t personally think, in the context of MaidSafe, that it’s because anybody here doesn’t want your help, or that they don’t appreciate the community effort (quite the opposite in fact!).

11 Likes

I haven’t re-read that PR discussion but have followed many here and elsewhere over the years and haven’t thought any of them were bikeshedding (which doesn’t mean that one wasn’t, because I haven’t gone back to look).

My observation is that almost all, and certainly the MaidSafe discussions are constructive and sensible. I often read them just to learn things including how to contribute to or maintain a shared codebase.

Having recently been on both sides, both of which are new to me I can see the difficulties. I both experienced the emotional side of being asked to make changes, and I think saw a contributor end up unhappy when I asked for changes as maintainer (though they haven’t said as much, so I’m left guessing).

In neither case do I think there was anything that should have been done or said differently, yet in both instances a contributor (me and probably a contributor to my code) had a difficult experience.

I am open to feedback, in case as maintainer I’ve missed opportunities to do better.

I didn’t feel the need to feedback to the maintainer wrt my PR (for wasm-git) because I don’t think there was anything he could have said better. But it was still hard to hear and respond when from his point of view he wanted and asked me to do more things before merging. I had not realised they would need doing and so was thinking my code would just be merged, me given a pat on the back and able to go and tackle the next thing on my list.

In the end I did the first change he asked for, or part of it, he helped so much that in fact it was really him. The second thing I didn’t want to do, and he accepted that, merged the code and left it as something which could be done later by someone else.

So I get both sides of this, and in particular how hard it can be for a contributor, particularly as someone new to this kind of collaboration like myself. As a maintainer, my experience as contributor meant I was trying really hard to be both clear and also not unreasonable, either in what I was asking our how I did so. Yet it still looks like that contributor was left feeling unhappy about the process. Although as noted I’m speculating because I haven’t receive feedback about how I handled this.

So @anon57419684, I get you! This can be hard for contributor and maintainer. As contributors we can put a lot of effort into something, and then have to go through a process of others looking at something we think is ready and they raise things we didn’t anticipate, some we may think are unreasonable etc. That’s a vulnerable situation, and I’m not sure how I’m going to handle it myself when I do get a maintainer who’s not experienced, not clear, or just not socially skilled etc. I think what I will try to do is first be objective, and then decide if I think it’s worth me offering feedback. I might just walk away, if I don’t think it’s worth my time. It will also depend on how important my contributing is to me in a particular case.

So I think it’s great that you found a way to give feedback, and that as I’ve come to expect the response from MaidSafe developers is receptive even if they don’t necessarily agree.

One thing I know is that both sides of this are much trickier than I’d imagined! So I have a new respect for maintainers and contributors, and I’m even more amazed and more grateful for all that they do. Something important I’ve learned is that it helps to be on both sides, as maintainer of some things and contributor to others.

Well done and thank you all of you!

9 Likes

I could well believe that!

Writing unit tests for interpreted, loosely typed languages like PHP is definitely a pain in comparison to compiled, typed languages like Java. If Rust takes this a step further, there must definitely be fewer edge cases to consider.

I hope to find out for myself in a few weeks!

5 Likes