Howdy,
I have been slowly working my way through WriteFreely from ground zero (as an admin and open source contributor) in the past few weeks so I think I could provide some input. My suggestions are solely based towards open source developer experience working with WriteFreely. I have primarily a golang background for context. This review is meant to be constructive and helpful (I am aware of the limited resources and longevity of the WriteFreely codebase).
While working on a bug fix for WriteFreely one thing that stands out to me about the codebase is readability. I think there is room for improvement. Following common go coding practices as well as increasing godoc (even for unexported functions) would be a good start.
When searching for this bug, I had to place random debug statements in each function in posts.go because it was unclear to me which function would be called when a user tries to access a post. I initially thought handleViewPost would be where the bug was, but to my surprise the bug was in viewCollectionPost. Neither function has documentation explaining the scope of their functionality so I’m still not entirely sure why this is.
This codebase has bus factor right now and I’m happy to see that there are ongoing efforts to address this. Increasing readability and continuing refactor efforts to keep the code DRY is going to be more fruitful effort in my opinion then building fancy dev tools. Already I can see a few main areas of developer experience that can discourage continued contributions. From my experience, maintaining an open source project is about getting new contributors and maintaining old ones. Perhaps the latter is the most difficult task.
Readability, ease of testing, and maintainability are my concerns at the moment. The lack of support for those factors would likely prevent me from making consistent code contributions.
Readability would involve code layout, documentation, coding style, a shorter files/functions. I personally don’t prefer having all the core files at the top level intermixed with the README.md and other files. I think a lot of the go files could be moved into a core/ directory or something. Maybe there is good reason not to do this, like package naming aligning with directory naming, but these sorts of decisions would be good to document somewhere. All functions should have documentation/comments so if I randomly browse a go file I can know what it does (even if the name implies its general action). This is very important for longer functions which may have a variety of side affects/reasons for returns. I think there are various linting/nits/coding practices that could be updated. One nit that stood out to me was grouping vars which isn’t done in posts.go. If I opened prs to update some readability, are they likely to be approved? I see this function (again no documentation) which looks like it converts a time to something human readable, but this functionality should already be supported by golang time? Also, this is a personal nit, but file lengths of 1500 lines (posts.go) should not exist unless they are autogenerated files like protobuf files. Instead I’d like to see posts become its own pkg/directory and a different file for each action related to a post (query, add, delete, edit). These are just some good habits I’ve learned from working with go engineers.
Ease of testing would involve an easy to use integration test suite. The bug fix I did was an easy to make logic mistake, which I could easily see being repeated again. The only way to test for it right now is manually, but logically it is an easy test case. I’m imagining a test function which can generate a base application, with some base set of users (admin, baduser, gooduser) with a base set of collections/posts. Then I should be able to generate some authenticated user (u == nil || u != nil) and test if the returned page is a 404 or the actual post.
Maintainability is very important because it also implies how easy it would be to add new features. When interacting with posts for silenced users, there is replicated code doing roughly the same functionality: get the silenced status for the owner of a post, if the authenticated user is not the post owner, return post not found. There are exceptions to this case, but there are at least a few duplicated sets of code. Modifying this functionality in the future would involve tracking down the code in various locations. Ideally you just need to modify a single funciton/single unit test.
I know the github repo issues is to be only used for bug reports at the moment, but I think this fractures the centralization of development. As an outside contributor it is harder to keep track of two separate feeds of discussions. I’d prefer to be able to open an issue discussing readability, testing, or maintainability on github. IMO forums should be used for discussing future features, vision, and community interaction. Technical development should be on the repo. Templates could be added bugs vs improvement/feature proposals. I think this would increase visibility of ongoing discussion and it’d be easier to track down previous discussion on the same issues/topics.
I hope this helps and I’m happy to work on trying to improve these areas. Just let me know what parts you think would get approved and which parts you would prefer not to modify/change at the moment (and maybe a reason why?).
Edit: I should also mention that I have been very pleased with my interaction with WriteFreely with regards to usability, ease of install, very clean minimal UI, great privacy practices, and supportive community members. It is fantastic that WriteFreely exists as an open source option and is still being maintained by WriteAs despite it technically being a competitor. I just would like to help make it even better!
