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

Tweak how options[:temporal] is handled #97

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

pnomolos
Copy link
Contributor

@pnomolos pnomolos commented Mar 4, 2020

I still need to write specs for this, but it changes how options[:temporal] is handled so that if you don't pass anything explicit it'll work fine (rather than right now, where if you don't specify temporal: true on a chronomodel-handled table it'll return it to a plain table).

Copy link
Contributor

@vjt vjt left a comment

Choose a reason for hiding this comment

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

Looks sensible, thank you.

@pnomolos
Copy link
Contributor Author

Looks like this project has begun to be maintained again, so I've merged upstream master. If this PR is still useful would it be possible to get it in the next release?

@tagliala
Copy link
Member

Hi, same considerations as the other PR. Any chance to rebase instead of merge and provide a failing test case?

@pnomolos pnomolos force-pushed the pnomolos/better_transparency_for_temporalness branch from 4ab3d25 to 6a2aa61 Compare September 12, 2022 17:23
@pnomolos
Copy link
Contributor Author

@tagliala I've finally come back to this, including adding specs and reworking some functionality. I hope it's good to go now, please let me know if you'd like any further changes

@tagliala
Copy link
Member

tagliala commented Dec 6, 2024

Hi @pnomolos,

Just a heads-up to say thanks because I've cherry-picked some of these commits and merged them into master to fix an issue with change_table

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