A few questions about dev etiquette and contributing

I’ve followed the VyOS project since it started and used Vyatta before that, for a variety of things - from quick and dirty VMs on customer tin to get them out of a jam, up to production carrier level stuff. It is easily my favourite NOS.

I made some efforts to contribute early on after the fork, but nothing that ended up producing code I wanted to show anyone else - VyOS was changing rapidly (moving to a Python config backend, far nicer than the Perl config glue I’d been hacking on) and work’s business requirements at the time were solved with more Cisco, so I let it go. There were a couple more false starts over the years since. I’m still roughly familiar with how the project fits together - although it has changed considerably, for the better. I’d like to have another go at helping out where I can.

I’ve already got a dev account and have been picking through tickets to find anything that looks easy enough or catches the eye to re-learn the project. I don’t have access to Slack, nor have I requested it, since I’m not a customer or a proven contributor (from the request form). However, I’ve got a few questions for the devs on how I should be going about things:

  • Is it OK to just pick up and post suggestions or patches in unassigned tickets? For eg, T6157. I didn’t create a PR for such a small change and likely need to have a short discussion about whether that fix is still working as intended. Would it be preferred to just create a PR even for tiny changes?
  • On T6383 it felt a bit like butting in on dmbaturin but I’d just spent a spare 10 minutes looking through the new config repo and I didn’t want it to go to waste; he’d also asked the question. Hopefully my input was welcome there. If someone’s actively working on something, I assume they’ll usually be assigned the ticket, so I won’t end up doubling up or stepping on toes?
  • I’ve noticed a few long-running tickets like T5049, where it looks like the original issue was already solved and I’ve commented with what I think should be the current way of making it work, but I’m not sure how to progress that - the ticket was logged over a year ago and I’m not sure if the vyos devs want to check older versions or other testing lash-ups
  • Another long running-ticket is T2942, where I’m reasonably sure the user was doing it wrong but would like to talk over whether that is correct and if it’s worthwhile making a bigger deal of it in the documentation.

I wasn’t able to find a forum post mentioning the ticket IDs or roughly matching the subjects for T5049 or T2942 to ask those questions here, so I left a comment in the tickets. After a week, I’m unsure if that’s the appropriate place or if they’re being monitored by the devs for those kinds of questions. Also, not trying to nag for a response - everyone is busy - just double checking I’m working through the correct process in a way that’ll be useful.

It is better to create a PR; in the PR, you will get feedback, for example, if something needs to change. Also, it shows if all requirements are passed, such as smoke tests.
If you won’t create PR, someone needs to create this PR anyway.
Always create a PR even if there is a small change like “typo” in the command description.

1 Like

@dmbaturin can bring more light on this

1 Like

No, please don’t do that — if you have a solution to a ticket, make a PR right away.

Our current process requires a PR and a review to get any change into the codebase (as in, even maintainers do not push anything to the repository directly), so someone will have to make a PR either way. We also look at PRs first to see if someone is eligible for a contributor subscription, so if you want to apply, creating PRs will make that easier — we’ll be able to see it just from your GitHub username.

On T6383 it felt a bit like butting in on dmbaturin but I’d just spent a spare 10 minutes looking through the new config repo and I didn’t want it to go to waste

We ended up recreating basically the same change there indeed. If there was a PR, I would merge it. Generally, if a task is assigned, it’s always a good idea to ask the assignee if there’s active work underway.

We are now working to improve the process so that if a task is assigned, it means that it’s actually worked on. We still have a lot of old tasks where the fact that it’s assigned means nothing… the plan is to automatically un-assign them after a set period of time if there’s no activity from the assignee. That will make it much safer to claim unassigned tasks.

I’ve noticed a few long-running tickets

If there’s a long-standing bug without a confirmation that it was fixed, we are really grateful to people who can test and confirm it. A lot of the time the original reporter just disappears without a trace…

We are also working to make tasks more granular and reduce long-running tasks.

Thanks for the responses guys. I’ll create PRs in future instead of sprinkling diffs everywhere.

The main thing I was coming up against with the long-running tickets was, a lot of them are less about a straight code fix and more about figuring out what is supposed to happen.

In T2942, I get what they’re trying to do, but the way they’re configuring it looks incorrect, I don’t think it can match that way.

For situations like that, should I just raise a thread in the dev forum to ask questions about how a feature was intended to work and potentially any additional context to the question/work that wasn’t in the ticket? To figure out whether the implementation/documentation needs tweaking or to just provide suggestions for an alternative config.

1 Like

I’m also unsure here. Thanks for your question @talmakion!

My first idea on this is to have it in the ticket, so every information needed is available from there.
Thinking twice, this might render tickets unreadable.
So maybe a thing in-between, put quick remarks in the ticket and put links in the ticket to forum threads with major discussion?

Maybe we can add a custom field to the vyos.dev task that contains a link to a forum discussion?
@dmbaturin @talmakion @alain, what do you think about such approach?


This would provide a more generic formal structure to tickets and make them easier to read than the stuff I usually cobble together in mine… :smiley:

Cross linking is a handy and quick way to bring together all discussion on the issue. The forum seems like a better place to collect opinions or renew a discussion on tickets that have gone stale.

I’ve got a short list of other tickets I’ve found in a similar situation, most of them have an obvious forum thread they came from, but not always. I’m still reading through the whys and wheres of the code involved for those ones before I stick my nose in and make suggestions.

Would be prefered if the discussion about a PR could be done at one location instead of (as its today) at three different places:

In order to FOMO (fear of missing out) one is (in worst case) forced to paste the same comment at three different places just to make sure that something isnt missed by others.

@talmakion @alain

@dmbaturin added a custom field to vyos.dev tasks
I will work with @evgeniy.bondarenko to add a custom field to forum topics (to see if we can have a link to the topic). Meanwhile, we can post a link to vyos.dev in the first post.

@Apachez, there is no universal solution. Depending on the issue, it may or may not require forum discussion. When everything is clear, and the task is to add some trivial stuff, no forum topic will be needed, but if an issue or feature requires discussion, a forum will probably be an easier option for many people who do not have vyos.dev account but want to provide input
Discussions in PRs are mostly related to code changes in PRs, not about the feature/issue itself


@talmakion as a fellow contributor wanted stop by this thread and give my appreciation to the work your doing (have seen you post a few PRs over the past days). Our community grows one person at a time :grinning:


Cheers mate - just learning by attacking a few bug tickets here and there, hoping to get to the point where I can contribute to features.