search

remix-run / blues-stack

The Remix Stack for deploying to Fly with PostgreSQL, authentication, testing, linting, formatting, etc.
https://remix.run/stacks
MIT License
944 stars 231 forks source link

chore: switch to toml-select github action #235

Closed sravinet closed 6 months ago

sravinet commented 7 months ago

Quick improvement See https://github.com/marketplace/actions/toml-select-toml-field-extractor-action

ryanhalliday commented 7 months ago

Why on earth should this be changed to a package you control? It's not like the old one is out of date or doesn't get updates: https://github.com/SebRollen/toml-action

Is this some cybersec theatre?

MichaelDeBoey commented 6 months ago

Since your action is literally doing the same thing as the one we currently use, I don't see why we would change usage tbh.

sravinet commented 6 months ago

Hey I appreciate the great work the team is doing with remix and these stacks.

But the language used to reply to my proposed improvement (PR) is a little harsh, would you agree? Not so sure you would talk like this is a real-life conversation.

If the maintainer's response is related to our humor with toml-select you have fully the right to not laugh or smile.

Language or humor aside, please recognize you didn't give me much time to explain. About 24 hours.

I wasn’t expecting the need to explain so deeply something as simple and not used in production executed code but in CI code.

You’re right in saying the GitHub action is doing the same thing as the one you are currently using. It is on purpose using the same API, to be functionally compatible, and an easy drop-in replacement. It is just doing the same thing very differently.

We worked on our own GitHub action when it was still using node16 (only updated in toml-action commit Jan 27, 2024). The previous toml-action commit was April 2023, with long-standing issues so we concluded it was not actively maintained. When we started improving toml-action we came to rebuild everything. Hence a separate toml-select project. I explained more on our Readme, it is easily overlooked.

Here's why switching to this toml-select GitHub action might still be a better idea than keeping the current toml-action:

Of course, we use toml-select in production, in our remix-based sites.

Now, you guys are maintaining Remix, we're not, so, of course, you do what you find best and it's very right.

We all try to do our best here, and share, even small improvements, one step at a time.

Enjoy the weekend.

ryanhalliday commented 6 months ago

I am not a maintainer, my opinions are my own. My harshness was to ensure this did not get merged without strong consideration as it has the potential to proliferate to a lot of projects.

Suggesting an open source project change to a package you maintain should come with much more explanation than "Quick improvement". I'm sure the contribution is appreciated but appears unnecessary in the current circumstances.

That's the last I'll say on this, sorry if my wording caused a minor headache maintainers.

sravinet commented 6 months ago

Sure. Thanks for the apology. Nothing was hidden Ryan. Link to full description was provided very start.