Skip to content

move indexing from Dash.jl #5

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

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

BeastyBlacksmith
Copy link
Contributor

Copy link
Contributor

@etpinard etpinard left a comment

Choose a reason for hiding this comment

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

Fantastic. Thanks very much!

@etpinard
Copy link
Contributor

etpinard commented Aug 2, 2023

I'll merge this, but I'll wait until #4 is done to release this PR along with #4 in DashBase v0.3.0

@etpinard etpinard merged commit 8abcb89 into plotly:master Aug 2, 2023
@BeastyBlacksmith
Copy link
Contributor Author

BeastyBlacksmith commented Aug 2, 2023

in DashBase v0.3.0

There are no breaking changes in any if those PRs, so this should be released as 0.2.1

If you were to make a breaking release, I'd vote for making that 1.0.0.

@BeastyBlacksmith BeastyBlacksmith deleted the bbs/index-by-id branch August 2, 2023 13:59
@etpinard
Copy link
Contributor

etpinard commented Aug 2, 2023

From the perspective of DashBase.jl, you're absolutely right. We're just adding things here.

But, what would happen if we release DashBase v0.2.1 off the current master, then users installing or updating Dash.jl would get DashBase v0.2.1 along with Dash v1.3.0 meaning that Base.getindex(component::Component, id::AbstractString) would be defined in two places. Doesn't that lead to errors?

If you were to make a breaking release, I'd vote for making that 1.0.0.

Yep, we can make the next DashBase release v1.0.0

@BeastyBlacksmith
Copy link
Contributor Author

BeastyBlacksmith commented Aug 2, 2023

We'd need to test that precompilation does not break, but I'd think that those methods would just get overridden, which is not ideal, since you'd get the buggy version then.
On the other hand that is only an issue until the new Dash version is released. But then, if Dash is the only consumer of DashBase, making it 1.0 is also fine.

@etpinard etpinard added this to the v1 milestone Aug 3, 2023
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