search

newjersey / taxation-mef-viewer

Fork of betson/irs-efile-viewer for DOT use
MIT License
0 stars 0 forks source link

[SHOW] Fixes for demo #6

Closed jachan closed 3 months ago

jachan commented 3 months ago

Description

Changed index.html landing page, downgraded nokogiri version. These changes were made to get the package in a working state for demos to stakeholders, committing them to make sure they are not lost.

Approach

Nokogiri downgrade is necessary as version 1.16.x was breaking for this package.

Steps to test

I tested this by running it locally with Jekyll using script/server and verifying that the index.html was updated appropriately and that the links worked.

Notes

aloverso commented 3 months ago

✅ Makes sense to me

Appreciate that the nokogiri change is a separate commit from the tax form update, that's a good separation of concerns imo. For the future, I might have recommended squashing the two tax form commits into one commit though, since they seem to be related or co-necessary for the same "feature"/need.

jachan commented 3 months ago

Thanks for the callout about commit management! I have previously been squashing all my commits into one per PR, but recognize that there is value in rebasing manually before merging, will try and do that for all future PRs.

casewalker commented 3 months ago

It's been like 12 years since I have seen a Gemfile 😭 🙏