jgarber / redcloth

RedCloth is a Ruby library for converting Textile into HTML.
Other
445 stars 102 forks source link

Add GitHub Actions - Ubuntu, macOS, & Windows, Ruby 2.4 thru head, misc updates #83

Closed MSP-Greg closed 8 months ago

MSP-Greg commented 12 months ago

See https://github.com/MSP-Greg/redcloth/actions/runs/7121117282

Some of the changes are 'general cleanup', some are Windows specific changes.

I believe this needs PR #82 merged for Windows CI to pass.

closes #63, closes #71, closes #81

MSP-Greg commented 12 months ago

As mentioned in #82, this started this morning, and I now actually looked at other PR's and issues. Added 'co-author' both here and in #82, and also added all the issues and pr's closed by them...

PR #74 should also be merged. Haven't looked at the others.

MSP-Greg commented 12 months ago

@digitalmoksha @heliocola

Sorry for the ping. I forgot that I was a collaborator here. I am busy enough with other OSS, and I don't know Textile docs.

So, I'm not sure if I can help with real code issues here, but the infrastructure things (build scripts, CI, etc) I can help with.

I think #74, #82, and this PR can be merged, and they'll give a good start to updating things, along with CI. I haven't worked with any CI going back before Ruby 2.4 for a while. I know Windows is really a PITA, not sure about macOS or Ubuntu. I did not add a required_ruby_version to the gemspec. Also, deleted the rvm stuff here.

So, wondering if you have any opinions...

MSP-Greg commented 12 months ago

Added Ruby 2.2 & 2.3 on Ubuntu & macOS. All four jobs passed.

heliocola commented 10 months ago

Hi there @MSP-Greg . Thank you for submitting this PR. I've just merged #81 that was queued up before yours.

Can you please sync your branch with latest master?

digitalmoksha commented 10 months ago

I don't see the actions running in this branch, so I pulled it into a fork. The Windows version is not building properly, but I think that's what #82 / #62 is supposed to fix.

Once this is rebased, I guess we can remove ruby.yml

Overall this looks pretty good!

MSP-Greg commented 10 months ago

I don't see the actions running in this branch

New workflow files never run in upstream from a PR. They will run in forks, because they're normally running from a branch.

Rebased and added Ruby 3.3. to the workflow. I'll wait until #62 or #82 are merged, as CI will fail until then.

The Windows version is not building properly, but I think that's what https://github.com/jgarber/redcloth/pull/82 / https://github.com/jgarber/redcloth/pull/62 is supposed to fix.

Yes, and I've needed to patch that every time I install RedCloth on my Windows system. I have a lot of Rubies installed on all my systems. Both #62 and #82 are simple PR's. #62 did not delete the code related to 'pre-compiled' gems, #82 did.

heliocola commented 10 months ago

I'll wait until https://github.com/jgarber/redcloth/pull/62 or https://github.com/jgarber/redcloth/pull/82 are merged, as CI will fail until then.

@MSP-Greg : I've merged #62 , can you please check if that suffice for this PR?

I spent some time reading your comments in different issues to situate myself on all the windows issues you've had in the past years (way before I even knew this gem existed).

This comment caught my eye: https://github.com/jgarber/redcloth/issues/51#issuecomment-375977071

It does look like there is a better way to do these extensions and nokogiri is a really good reference. Thank you for pointing that up, I will try to spend some time studying nokogiri's code.

As I've mentioned in a few other comments I am still trying to situate myself so I don't break anything... :-D

digitalmoksha commented 10 months ago

It does look like there is a better way to do these extensions and nokogiri is a really good reference. Thank you for pointing that up, I will try to spend some time studying nokogiri's code.

I think the common way of doing this is with https://github.com/rake-compiler/rake-compiler.

MSP-Greg commented 10 months ago

rake-compiler is in the Gemfile and is being used.

MSP-Greg commented 10 months ago

I just rebased, added Ruby 3.3, and made use of a new feature in setup-ruby.

All passed, see https://github.com/MSP-Greg/redcloth/actions/runs/7590837155/job/20678021321

Obviously, it takes longer than the current CI, but it tests all platforms. Note there are some warnings thrown in the compile step...

heliocola commented 10 months ago

Thank you @MSP-Greg .

I will look further in the gem release process and perhaps work on a clean up PR that I've been planning. I was trying to avoid having to look into that before closing these issues, but perhaps is better that I work on it first.