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

Move Global State Init to an Attribute-based system #89

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Fayti1703
Copy link
Member

What it says on the tin.

Open Issues:

  • These changes are not tested.
  • There may be problems with ordering of the initialization.

Because I don't really trust the ordering to not become a problem, I'm marking this PR as a draft until it is field-tested.

Relations:

@Fayti1703
Copy link
Member Author

Fayti1703 commented Dec 27, 2022

@emyka reported that there currently don't appear to be any ordering issues in the existing initializations.

I would still like to add an InitAfter field/property to GlobalStateAttribute prior to merging this, however.

@Fayti1703 Fayti1703 force-pushed the proposals/fayti1703/auto-global-init branch from 3769631 to 7c0d019 Compare January 1, 2023 15:45
@Fayti1703 Fayti1703 marked this pull request as ready for review January 3, 2023 13:59
@Fayti1703 Fayti1703 requested a review from chirpxiv January 3, 2023 13:59
First to init, last to dispose; for dependency reasons.
New system is going to follow this order as well.

This commit exists primarily for bisection reasons. If a bisect lead
you here, you just discovered an ordering problem.
Note that the individual initializations are
currently **entirely unordered**!

This may or may not present a problem later down the line.

If a bisect lead you here, it does/did present a problem.
@Fayti1703 Fayti1703 force-pushed the proposals/fayti1703/auto-global-init branch from 7c0d019 to 3537188 Compare January 12, 2023 07:49
@Fayti1703
Copy link
Member Author

Rebased on top of current main and resolved conflicts.

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.

1 participant