Add back workflows

[cewert]

Using the viv repo means we no longer have any automatic builds or linting. Why remove them? Are there any plans to bring them back?

7c68c73

[jimdogx]

This isn't appropriate as a Github issue. This is more of a discussion and better suited for either the Jellyfin Forums and/or the Matrix.

FYI though: The Legacy app workflows needed some cleanup anyway. So now we've got a clean slate. 👍

[cewert]

Just noticed you also removed the npm scripts to run the rooibos unit tests. Why remove the unit testing? I'm so confused. It feels like we're going backwards.

old https://github.com/jellyfin/jellyfin-roku-legacy/blob/master/package.json

new https://github.com/jellyfin/jellyfin-roku/blob/master/package.json

[cewert]

Feel free to move this to a github discussion than. What issues did the workflows have? They worked fine for me.

Edit: The workflows are still running correctly on the legacy repo. What problems were you guys having???

[cewert]

The npm scripts that validate our brightscript code were removed? One of them is STILL mentioned in the dev readme but no longer exits in the package.json file https://github.com/jellyfin/jellyfin-roku/blob/master/docs/DEVGUIDE.md#committing

Some of these npm scripts were setup by the guy who wrote brighterscript so I'm super confused why they were removed.

jellyfin-archive/jellyfin-roku-legacy@ecf4090

[cewert]

Are any of these problems worthy of a github issue? I will create new issues if it's appropriate. There was a lot to unpack so this issue became a mess :)

• automatic builds no longer work ( makes testing code easier for non devs and devs alike)
• linting all code for PRs no longer works (this has prevented app crashes by catching translation files with invalid XML not to mention it was enforcing code standards on our entire repo)
• unit testing no longer works and appears to have been completely removed with no replacement
• npm scripts to help devs find bugs before submitting a PR has been removed with no replacement
• docs still referencing old npm scripts
• code documention no longer exists with no replacement (https://jellyfin.github.io/jellyfin-roku-legacy/)
• Beta Test instructions were removed. This was the easiest way for non devs to sideload the app and was used often

[jimdogx]

None of these are "issues". Issues are things like "Roku App doesn't let me scroll to see the last row on the home screen". Everything you mentioned are developer enablement items. Things we do to make developer lives easier (not the end user).

Unfortunately you've been away for a while, but to bring you up to speed. v3.0.0 is a huge refactor. Not everything was ported over (but it was worth it given how many new features, security improvements, etc v3.0.0 brings).

I'm not saying we won't add Unit Tests or NPM scripts back in. There just isn't a rush to do so at the moment. All of our efforts were on getting v3.0.0 out to folks who have been asking for a bunch of things over and over (lyrics, trickplay, media segments, etc). Then we had to pivot to support a few folks that simply can't upgrade their 2011 computer and our stuck on old, insecure versions of the Jellyfin server.

[cewert]

Completely disagree. Helping the devs helps the end users indirectly - the more devs that help and the faster they finish PRs the happier our users are. Linting the code to prevent app crashes helps the users directly by preventing their app from crashing. Giving them instructions to sideload helps them directly.

So in order to push out 3.0 these changes were necessary? Or that removing all of this was part of a refactor? They were removed Sep 26, 2024, WAY before the 3.0 release. You could literally add back most of this now and there would be no downside.

Yea, I've been gone for a few months, but all of the workflows worked fine when I left. No complaints from you or anybody. Now all the sudden they were removed because they didn't work right? It just so happens that I was the one who wrote most of the code for all the things that are now missing. Could you please create an issue explaining the workflow problems you guys are having and maybe I can help get them running again? Unless you guys don't want to use them anymore 🤷 I assume there would be an issue for it if it was on yours guys radar

[cewert]

When did we stop allowing issues for dev related things? Is that something that changed with 3.0? We had lots of these on the old repo. We even had a tag for it and everything but now you're telling me they aren't an issue? We had one for the change to brighterscript. One for adding unit testing. One for workflows etc.

I thought issues were meant for issues with the entire repo and it's code base but apparently not? Might want to add that to the issue templates 👍

[cewert]

Do you want help fixing them or not? I'm willing to help but I need to know why they were removed and what problems you guys were dealing with if any. Testing workflows is a PITA

Some of the NPM scripts could be added back right now with zero downside but it doesn't seem like you guys care about them getting fixed. I don't want to waste my time making a PR if you guys are just going to reject it.

[jimdogx]

Do you want help fixing them or not?

Sorry, I hope you weren't expecting immediate responses on a Tuesday. I'm at work.

I don't want to waste my time making a PR if you guys are just going to reject it.

I would hold off on any PRs. The repo is in flux right now. As I said before, priority was getting v3.0.0 out the door.

[cewert]

It's a yes or no question I don't understand the problem. A bunch of dev stuff got removed in 3.0 for some reason and this doesn't bother you because you get to "start fresh" even though the repo is more prone to having bugs. I'm the one who wrote the majority of the code and I'm willing to fix it (if it was broken??). You guys don't have time because you are too busy fixing bugs. I thought this would be an easy yes for you and then everyone wins but I guess not.

How does the repo being in "flux" effect some of the specific things I'm talking about though? That's the part I can't wrap my head around. We can add back npm validate for example and there's no way that would affect anything. Same with npm lint. These will immediately improve the dev workflow and fix the now broken docs. Small PR. Easy review. Where's the problem exactly?

I have an idea... what if I make issues for these things in the meantime? That way we have a place to track them and discuss their (re)implemention. It would be a place for you guys to tell me about any problems you ran into. It would also guarantee these things aren't forgotten about in the middle of this repo fluxxing and allow you guys to track them and put them on project boards in the future. That seems like the next best thing yea?

PS: it's possible the current code has bugs that would have been caught by the CI/npm scripts. When I imported 1hits roku npm modules into our code base there were several bugs that linting would have caught but he wasn't using one. jellyfin-archive/jellyfin-roku-legacy#1247

[jimdogx]

There's duplicate actions in the Legacy app. Some run, some don't. Lint hasn't run in 6 months 🤷

Again, just saying we didn't want to bring it over as is. It needs some cleanup.

[cewert]

Nothing you've mentioned is an actual problem that prevents the workflows from running or even needs fixed. You're looking at a list of ALL workflows that have ever ran on the repo. Those are not all active workflows or even files that currently exist in the repo.

lint workflow hasn't run in 6 months because I deleted the file the last time I reworked the workflows. Instead of one giant workflow linting ALL of our code, I updated the workflows to only validate code that has changed. i.e. only run bs linter when bs/brs files change. That's why there are now lint-brightscript lint-json etc. And those workflows have been working and running fine for the past 6 months. Look at my recent PR for an example of them working as they should.

Why the duplicates? Did you not look at the files at all?? Based on the docs I read, an underscore at the beginning is a way to say the workflow is reusable. This was another change I had to make in order to improve the workflows. Some things needed to be run with multiple triggers so instead of duplicating code, I created a reusable workflow and then just have the separate triggers call it. You even approved this one jellyfin-archive/jellyfin-roku-legacy#2063

Any other examples of things that need "cleaned up"?

Edit: Some run, some don't. Not all of the workflows are automated. Some have manual triggers. Some aren't "running" because they are running the reusable workflow. Create PR to bump version is an example of a manually triggered workflow. dependencies is triggered by a PR label IIRC so that we could test renovate dependency PRs before merging them (this has broken our app in the past by not testing renovate PRs).

[jimdogx]

I started to reply to each item, but the TLDR is I want to do it differently this time.

But thanks for the offer.

[cewert]

Hey we're finally getting to the truth. This is great. So the workflows I made work fine, yea? They worked fine 6 months ago and they work fine today, correct?

What is it you want to do differently? I would love to help brainstorm and come up with a solution that works for everyone. You could make a github issue for this and we could discuss your pain points and ways we can improve things?