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

Prepare for csize to be changed to uint #121

Merged
merged 1 commit into from
Oct 1, 2019
Merged

Conversation

Clyybber
Copy link
Contributor

@Clyybber Clyybber commented Oct 1, 2019

@c-blake c-blake merged commit d95682e into c-blake:master Oct 1, 2019
@genotrance
Copy link
Contributor

Can we get a release for this?

@c-blake
Copy link
Owner

c-blake commented Oct 8, 2019

Yeah. I should fix up a few other places that need similar adjustment. Give me a few minutes.

@c-blake
Copy link
Owner

c-blake commented Oct 8, 2019

Ok. New release published. Let me know if you have any problems. I was just running into these csize problems this morning myself.

c-blake added a commit that referenced this pull request Oct 23, 2019
@c-blake
Copy link
Owner

c-blake commented Oct 23, 2019

I guess there's been some thrashing about on this. Maybe I should not have fulfilled @genotrance's request for a new release. In my defence, I think knowing what to do 15 days ago would have required clairvoyance about how Araq was going to handle things. Anyway, I think I am going to just define csize as a module-internal type alias for uint in all the modules that use it. Objections? Affirmations? Alternatives? I just pushed a commit. Speak up today before I re-release.

@Clyybber
Copy link
Contributor Author

I'm sorry, I couldn't have predicted that the change would be reverted (and tbh that revertion was pretty useless, as this change didn't cause much more breakage than what I've send PRs out for)..
RE: csize as an internal alias: I think thats a good idea.

@c-blake
Copy link
Owner

c-blake commented Oct 23, 2019

Ok. It's only a few lines and not drastically confusing. Maybe a year from now, I'll drop support for old Nim versions. Then the story will be simpler.

I do think deprecation periods are nice, and Nim does have mechanisms for that, though types are trickier than overloads on procs. Transition care is always a bit of a judgement call. Anyway, I'm just trying to do right by cligen users going forward not relitigate some Araq decision. Just bore mentioning that I couldn't know the right thing to do.

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.

3 participants