Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 148: add newtype for column and row indices. #161

Merged

Conversation

LukeTemp
Copy link
Contributor

This PR adds a newtype for column and row to improve the type safety and readability of the code: #148

@LukeTemp LukeTemp marked this pull request as draft August 31, 2022 13:35
@LukeTemp LukeTemp force-pushed the issue-148_add-newtype-for-cols-and-rows-2 branch from 88f6356 to 4eaaa0b Compare August 31, 2022 14:27
@LukeTemp LukeTemp marked this pull request as ready for review August 31, 2022 14:29
Copy link
Owner

@qrilka qrilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to me and mostly a mechanical one but would you mind fixing CI failure?

src/Codec/Xlsx/Types/Common.hs Outdated Show resolved Hide resolved
@qrilka
Copy link
Owner

qrilka commented Aug 31, 2022

Also it looks like the merged in #158 brought some conflicts with this one

@LukeTemp LukeTemp force-pushed the issue-148_add-newtype-for-cols-and-rows-2 branch from cd50f32 to af848aa Compare September 7, 2022 10:15
@LukeTemp LukeTemp force-pushed the issue-148_add-newtype-for-cols-and-rows-2 branch from af848aa to 439fae7 Compare September 8, 2022 10:07
@LukeTemp LukeTemp requested a review from qrilka September 8, 2022 10:16
test/StreamTests.hs Outdated Show resolved Hide resolved
@qrilka qrilka merged commit ad3bfe1 into qrilka:master Sep 9, 2022
@qrilka
Copy link
Owner

qrilka commented Sep 9, 2022

thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants