Skip to content

Added GetFormItems()#298

Closed
mikeschinkel wants to merge 1 commit intorivo:masterfrom
mikeschinkel:form_getformitems
Closed

Added GetFormItems()#298
mikeschinkel wants to merge 1 commit intorivo:masterfrom
mikeschinkel:form_getformitems

Conversation

@mikeschinkel
Copy link
Copy Markdown
Contributor

As I could not find a current way to determine the number of form items from code written outside of the tview.Form class, the GetFormItem(n) method can't be used reliably if the number of fields is unknown. For my use case I wanted to iterate all of them so I am submitting this pull request that would allow a developer to do so.

Hopefully this is simple and obvious enough of a change that it will be a no-brainer to commit.

(Note: This is a replacement for #287 that I closed because it was based on the master branch and not on a fork specific to this PR.)

@mikeschinkel mikeschinkel mentioned this pull request Jun 9, 2019
@rivo rivo closed this in 9c225ec Nov 27, 2019
@rivo
Copy link
Copy Markdown
Owner

rivo commented Nov 27, 2019

I prefer to add GetFormItemCount(), which I did in the latest commit, instead of exposing Form's internal item slice. According to your description, this should also solve your problem.

@mikeschinkel
Copy link
Copy Markdown
Contributor Author

@rivo Interesting. Okay, I will have to check to see if that addresses my needs. Not sure if once I started using the slice I did not use it for other things.

Unfortunately though, that project had to sit on the shelf as other priorities emerged, so it might be awhile before I can check.

Whatever the case, I would really like to be able to continue using your version instead of my fork, so hopefully your change will meet my needs. (I am assuming of course you are able to apply or offer a work-around for my other pull requests.)

@rivo
Copy link
Copy Markdown
Owner

rivo commented Nov 27, 2019

Pull requests cost by far more time to evaluate than issues. I wrote in the contributing guidelines that contributors should open issues before submitting PR's but it seems that most just ignore it. As a consequence, many of them will have to wait. If they're large PR's, I don't even know if I will ever get the time to look at all the code.

So it may take a while before I can give an answer to your other pull requests.

@mikeschinkel
Copy link
Copy Markdown
Contributor Author

@rivo — I did previously notice your comments in your most recent commit, and I wondered if they targeted me. But then I looked to see that there are a lot of other pull requests too so felt less targeted.

But I do hear you and respect your position, especially since you have made this code available for free for others to use and review PRs requires time you may not have or may not want to spend.

That said, I hope you will entertain hearing the justification for PRs from my perspective.

  1. Like you I have time constraints so rather than wait until later when I might not have the time, it makes sense for me to create the PR now when I do have the time.

  2. I did explain the pull requests in each PR which is effectively that same as explaining the use-case in an issue. So if you do not have time to review the PR you could at least reply with your thoughts about the concept; no different than you would for an issue. The plus being if you agree that my approach was good you could then just apply the PR as opposed to having to wait for it, which might not come for a long time.

  3. Two of the five remaining pull requests I submitted are three lines long and just expose access to private properties; specifically Form Title and FocusedElement. I would think it would be easier to review that code than first have a discussion about it.

  4. But most importantly, I created my pull requests not because a burning desire to contribute to an open-source projects but because my PRs removed blockers for my use of your package that I needed immediately. So I implemented them for my own use and then I created a PR to share with you and others in hopes that you would incorporate them and I could then revert back to using your package instead of continuing to use my fork.

  5. Following on from scollable -> scrollable #3, if I did not create the PR at the time I implemented for my own use, it would become much harder for me to create a pull request from them in the future. So while you see PRs as your burden, I see them as minimizing the burden of my contributing back to your project.

For me, when I am working on a project that uses your library and I have a blocker that requires me to extend your package, I am going to do it then and there and it makes sense for me to submit that PR to you then, otherwise I might as well not worry about contributing back.

Question? Do you want contributions (from me) that can only come in the form of a PR, or would you rather me not bother you with them? If the former is the case I will be happy to contribute PRs along with an intro discussion for your consideration.

But if you'd rather not be bothered with PR than it will just be easier for me to diverge my fork from your library and start maintaining an alternate. Please understand I really do not want to diverge and maintain, but the only realistic way for me to try to maintain compatibility is to submit PRs as soon as I fix a blocker.

So if you want to discourage getting any PRs prior to discussion — unless you are willing to typically discuss within 24-48 hours — I will just focus on my fork as I do not want to annoy you.

@rivo
Copy link
Copy Markdown
Owner

rivo commented Nov 28, 2019

I realize I made it sound in my comment that this was about you specifically. It wasn't at all. This PR just happened to be one I resolved first so my comment landed here.

I totally understand that you'll want to make your own changes so your own projects don't get stuck. Of course, you're free to submit those changes.

I'll say a few things about how I approach this project. I hope this will explain how PRs will be dealt with.

First of all, I was caught off guard by the success of this package. I certainly didn't plan it. But I decided to keep it going and maintain it in the long term. (I find it frustrating to work with a project that ends up being unmaintained. At my company, we have unmaintained projects in production and they're a ticking time bomb.) Having said that, in all the things I do in my life (and there's a lot of things I'm doing), this project is quite low in priority. It doesn't generate any income for me and, unfortunately, reviewing issues and PRs is also not much "fun". I understand that it's necessary so I'll do it but to be honest, I'd rather work on some cool new features (I have ideas for this package) than to look through other people's code changes. Your changes may be small and that (sometimes) makes it easier to deal with them. But some other people submitted large PRs which will cost me many hours to review. (I had to chuckle a bit when I saw this comment.)

So I go through issues first and then PRs. The issues alone could keep me busy for a few days as some of them are not small. An issue like #320 requires in-depth study of the ANSI specification, which I actually did but I still made mistakes apparently. And some issues like #357 can actually be bugs in other packages by other maintainers so now I either contribute to their project, wait for them to fix things, or even implement a completely new project that solves that problem. That's why a PR can remain in the queue for a long time.

Unfortunately, I haven't seen many PRs which I could simply merge. My standards are quite high when it comes to coding practices. And I also want this package to be consistent. (It's already not as consistent as I'd like it to be, I don't want to make it worse.) In addition, I also have to judge changes based on how they might affect future features which I plan to introduce. A change may be a quick fix for someone's own project. But it could lead to trouble down the line in other projects that use tview. Lastly, I'm the one who ends up maintaining this code. I have to be 100% behind it, understand it 100%, and be able to make changes to it later if necessary. A contributor can disappear any time (and many of them do). I'm the one responsible for these contributions.

I aways wonder if I should reply with extensive feedback and ask contributors to make changes to their PR and resubmit. Or if I should simply go ahead and make the change myself how I want it. Because I don't have time to do multiple rounds of feedback, I often make the change myself (like in this PR).

Ok, so none of this is specific to tview. The creator of Redis has written a pretty good blog post which describes a lot of the things I experience here.

Regarding your question: Go ahead and submit your PRs. I will get to them eventually but be prepared that it may take a long time before I can take care of them. tview will keep progressing in a slow but steady pace.

@mikeschinkel
Copy link
Copy Markdown
Contributor Author

@rivo — I absolutely get where you are coming from. I would not like to be in a position where I launched a side project and it took off and demanded lots of my time, so I can definitely empathize. My sole goals for commenting is to try to find out what is workable, for both you and for me.

And thanks for your lengthy reply. I really respect that and respect your position, and appreciate you taking time to understand my perspective.

Not sure when I can get back to my project that uses tview — it is high on my list of things I want to do but lower on the list of things I must do, but when I can work on it I will, and I will probably be submitting some more PRs if I do.

That said, I totally understand if you don't get to them (quickly.) I already understood, which is why even though I was actually hopeful to get some feedback I did not ping you about them.

As for code quality, there we want to same things. I don't care that if it is code that I wrote the gets merged vs your code, I only care that my use-cases are unblocked. But having quality code to depend on is certainly a good thing in my book, vs. a package that becomes a rats nest of indecipherable code.

P.S. Maybe you could identify one person who could do the review of the issues and PRs and then meet with them occasionally via screen share to provide them guidance? They do the heavy lifting and you oversee the architecture. I do that for one of my projects and it works well. But I have to pay them and I bet you could find someone who has submitted a pull request to do for free. Something to consider...

@rivo
Copy link
Copy Markdown
Owner

rivo commented Nov 29, 2019

Thanks, yeah, that could be an option.

ps. Btw, fun fact: Every time I reply to an issue, a whole bunch of new ones get opened. Someone sees that I'm active so they use the opportunity to engage me even more. It never becomes less. ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants