Rakefire / jekyll-ical-tag

Pull ICS feed and provide a for-like loop of calendar events in jekyll's liquid tag
7 stars 6 forks source link

`pry` Gem is included in production builds by default #22

Closed fabacab closed 4 years ago

fabacab commented 4 years ago

Is pry really necessary for a production build, or can it be limited to development only? See line 37 of jekyll-ical-tag.gemspec. I'm not certain one way or another but just noticed my build logs still including this Gem on my servers, and if my understanding of pry as a debugger/repl is correct, then it should probably not be on a production machine, yes?

rickychilcott commented 4 years ago

Definitely not necessary. I'll investigate whether I can remove it and make it just a dev dependency. IIRC I moved it to a non-dev-dependency because I don't have a working test suite for this plugin and trying to debug using a jekyll build (in another directory) wouldn't include it. But I could be wrong on that.

I can also just use binding.irb when I need to debug since that's built into ruby core.

fabacab commented 4 years ago

Definitely not necessary. I'll investigate whether I can remove it and make it just a dev dependency.

That would be super appreciated as, at the moment, I think including pry on production would be classified as CWE-489 Active Debug Code and could pose a risk to our CI/CD systems.

rickychilcott commented 4 years ago

FYI, latest release has this removed. Thanks for highlighting this as an issue

fabacab commented 4 years ago

Wonderful, thank you for updating!