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

Add dominance algorithm implementation #326

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

yamadapc
Copy link
Contributor

@yamadapc yamadapc commented Feb 5, 2025

Implements "A simple, fast dominance algorithm", to find the immediate
dominators of all nodes in a graph.

This will be used for a new experimental bundler package to be added.
This adds the empty package.

Test Plan: Run yarn test to execute unit-tests for the implementation

Created using spr 1.3.5
@yamadapc yamadapc requested a review from a team February 5, 2025 04:31
@yamadapc yamadapc enabled auto-merge (squash) February 5, 2025 04:31
@yamadapc yamadapc disabled auto-merge February 5, 2025 06:25
yamadapc added a commit that referenced this pull request Feb 5, 2025
Implements "A simple, fast dominance algorithm", to find the immediate
dominators of all nodes in a graph.

- https://www.cs.tufts.edu/comp/150FP/archive/keith-cooper/dom14.pdf

This will be used for a new experimental bundler package to be added.
This adds the empty package.

Test Plan: Run `yarn test` to execute unit-tests for the implementation

Pull Request: #326

import assert from 'assert';
import {ContentGraph} from '@atlaspack/graph';
import {simpleFastDominance} from '../../../src/DominatorBundler/findAssetDominators/simpleFastDominance';
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason you're using a test folder instead of placing it with the source?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean?

Do you mean using

  • /src
  • /test

?

That's just how the repository is setup. I'd rather colocate the tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, colocation. Be the change...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we could do that for the entire repository separately (?)

@yamadapc yamadapc merged commit 888b09f into main Feb 6, 2025
18 checks passed
@yamadapc yamadapc deleted the spr/yamadapc/add-dominance-algorithm-implementation branch February 6, 2025 00:57
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