Suggestions for better plugin maintenance

I have two suggestions which I feel they are important and essential for better future;

  1. When we are facing a problem with any plugin, the first question is “What other plugins you are using”. That’s logical request to see if any conflict. If you have one or two plugin installed, then its easy to debug this idea of conflict possibility by disabling them. But if there is 5, 10 or even more plugins installed, this is not that easy to find the conflicting plugin. My suggestion to plugin coders/testers/advisers is to install all the available unbroken plugins in there development/testing environment to test/approve their readiness before share it with the world.

  2. Whatever the last change happened at Discourse update, most of the plugins are giving the Uncaught TypeError: Discourse.ajax is not a function. Using this as an example for now; I think (if its not already happening), these types of major changes had to be announced with an “URGENT WARNING: CALL FOR COMPATIBILITY UPDATE” to all Discourse Plugins coders/producers at the moment of such important change decisions!. Otherwise many of us loosing a lot of valuable time reporting and rebuilding for each plugin.

Please correct me if any wrong conclusion I have regarding my observation on the above subjects.

Discourse is a great infrastructure backed with fantastic community. I love it and Its easy to love :sparkling_heart: . Just some small suggestions I made to make life easier for everybody! I hope :wink:

Discourse.ajax was actually removed back in July, and I had a deprecation notice in place that said “Discourse.ajax is deprecated. Import the module and use it instead” - generally that is the process I follow: I add backwards compatibility, output the notice to the console, and then in the next release I will remove it.

On top of that, technically speaking Discourse.ajax is a private API in the sense that it was never part of the plugin API we exposed. There’s no way for me to know the countless ways people can tap into our private APIs, so it’s hard for me to know what would be considered urgent.

Personally I don’t think it’s much to ask of plugin developers to check the consoles for their plugins once every 2-3 months and fix any problems that come up. That deprecation notice had been in stable a long time!

6 Likes

That’s how I did it for mine, back in July…

The only thing that caught me this time around was the removing of string-buffer which was replaced with buffer-renderer. It wasn’t overly difficult, but I also don’t recall any deprecation messages for that in particular.

Granted, what would be really useful is a way to prevent loading a plugin unless the Discourse version is less than or equal to X. That way when Discourse updates to 1.8, my plugin will cease loading until I update it claiming 1.8 is safe.

I do understand that isn’t on the roadmap anytime soon though.

5 Likes

That’s definitely not something that will scale. Do you think WordPress developers have every plugins installed when they work on their plugin?

While we’ll do our best to make sure not to break #official plugins, we can’t (and won’t) do that for every single plugins available out there.

2 Likes

That’s fair and understandable, and what I meant by all the plugins, the plugin which is #official plus the popular ones. I don’t think it would be much more than 20 plugins. WordPress example is way far from Discourse case I believe. At least plugins quantity wise.

At the end. I just wanted to grab some attentions of people who is more experienced than me if they could do anything about it. And I have the believe that things could be done a bit smother with my deep appreciation for what they already doing!

Thank you for your nice answer and happy to understand again that Discourse are well managed. Thank you for your time explaining it to me.

I was just shocked by the quantities of plugins effected by the same reason at almost same time. And when I’m talking about the effected plugins, it’s includes some official ones like Discourse Canned Replies! which is still giving that error!

1 Like

:laughing: yep canned replies is still erroring @eviltrout. :wink:

Yes, I shouldn’t have posted so hastily. I did not realize how many of our own blessed plugins also broke.

Obviously I can’t expect 3rd party plugin authors to not make the same mistakes we do!

In the future, I will log all deprecations as errors so that they go into /logs so that we will catch them much faster. There’s no good excuse for having missed all this code for 3+ months. Sorry!

13 Likes

What could help here is if changes to libraries, i.e. functions in discourse/lib and discourse-common/lib, are treated with a bit more caution and notice than other changes. Typically it is a change to a commonly used library that catches one of my plugins, the most recent example being the change to buffered rendering that also effected @cpradio.

Generally speaking, I am confident that things are moving in the right direction in this respect. Just a few tweaks to how more systemic changes are notified are needed perhaps.

5 Likes

It doesn’t look to be a “hot” topic for @plugin_authors .

But I’m wondering if plugins had some type of “if tests fail - exit and continue” thing in place it would help to at least keep any problems contained to the problem plugin(s)

1 Like

Definitely this. I believe the root cause of this issue is because most of our plugin do not have client side acceptance tests in place. Something worth noting is that we’ve started to run the tests for some of our officially supported plugins and we’re working on adding the rest to the build.

https://github.com/discourse/discourse/blob/master/.travis.yml#L41-L42

1 Like

Sorry, but how would acceptance tests solve the fact that string-buffer was removed/replaced with buffered-renderer? Anyone who would have upgraded to latest, would have bombed regardless of tests. Same with Discourse.ajax. The fact it was deprecated wouldn’t have stopped it from passing tests and then immediately failing when it was officially removed. Any that would have upgraded their core Discourse instance would still have had failures.

The only way that would help anything is if Discourse when bootstraping ran the tests before permitting the plugin to be pulled in and thus fail the upgrade if the tests didn’t pass or not load the plugin. I don’t see that happening anytime soon. It would likely be easier to support the plugin defining versions it is compatible with instead.

2 Likes

Having tests wouldn’t have helped me with Discourse.ajax (as the location of an import changed), but it may have helped me with buffered rendering, as the essential problem was that the change in the render logic mean’t that my custom logic wasn’t being run, which tests may have picked up.

I think both points are valid.

Plugin authors, particularly myself, are often feature-driven at the expense of stability. It’s fun to experiment with features. It’s less fun to concentrate on stability. Experimentation is partly what you want from an open source community. But you also want 3rd party plugins that don’t themselves break, or break an entire instance, all the time. Ideally there’s a balance to be struck.

discourse/discourse has gone through a number of structural changes in order to keep the codebase up with current practices and performing optimally. I wouldn’t want that to change. I would just tweak how some of these changes are communicated before they happen.

I sum, I reckon two things could help here:

  • A clearer way of communicating upcoming changes that are likely to affect plugins.
  • More effort on the part of plugin authors to bake in stability, like adding tests.
6 Likes

I want a dead-easy, well-documented way to set up CI testing for my plugins, running within a discourse instance.

Babble has a robust back-end test suite, which is cool, but ideally I’d like every push I make (and maybe some of the pushes Discourse makes?) to do the following:

  • Pull in the latest Discourse (or tests-passed, probably)
  • Install my plugin on it
  • Run the plugin test suite from within the that environment, on Travis or CircleCI or whatever

This way I can at least guarantee that hitting all of my endpoints works as expected.

A next step could be allowing us to run JS specs from within our plugin as well. In an ideal world, we’d have e2e integration testing too (whatever the Ember flavour of protractor is; maybe it’s protractor?), which would allow me to further guarantee stuff like ‘the icon is showing up correctly’, ‘clicking on it does this thing’, etc, which would give me pretty high confidence that everything was okay given that those were passing.

4 Likes

It wouldn’t help with deprecation but I was addressing the problem where plugins blew up without us knowing. For our official plugins, we are working on running the tests for all the plugins on travis. Removal of Discourse.Ajax or string-buffer would have caused acceptance tests to fail and alerted us of it.

2 Likes

This is already possible and all you need to do is follow the right folder structure. For an example, have a look at

https://github.com/discourse/discourse-spoiler-alert/tree/master/test/javascripts/acceptance

5 Likes