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

deprecated test #423

Merged
merged 5 commits into from
Dec 31, 2021
Merged

deprecated test #423

merged 5 commits into from
Dec 31, 2021

Conversation

Joeysantoro
Copy link
Contributor

@Joeysantoro Joeysantoro commented Dec 30, 2021

Adds deprecated contracts signoff category for proposals_config.

Adds an integration test to require that these contracts are in fact deprecated and not contained anywhere in dependencies map

@@ -23,7 +23,17 @@ describe('e2e-dependencies', function () {
for (let j = 0; j < contracts.length; j++) {
const contract = contracts[j];
const category = addresses[contract].category;
if (category === 'External' || category === 'Deprecated') {
if (category === 'External') {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should make an enum for these categories in the types file and import it here instead of using string literals

Copy link
Contributor

Choose a reason for hiding this comment

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

(and also probably use said enum everywhere else if you can do that without manually editing 500+ lines of code)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it already was an enum. Used it here

}
}
});

it('all dependencies bidirectional', async function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this wasn't an edit on this pr but the string "Check dependencies all dependencies bidirectional" doesn't tell me what it's checking, maybe something like "Check dependencies are listed bidirectionally" would?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed this and others

kryptoklob
kryptoklob previously approved these changes Dec 31, 2021
@Joeysantoro Joeysantoro merged commit df2ceb7 into develop Dec 31, 2021
@Joeysantoro Joeysantoro deleted the feat/deprecated-check branch December 31, 2021 05:01
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