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

Remove unnecessary tsconfig arguments #798

Merged
merged 2 commits into from
May 7, 2021

Conversation

snario
Copy link
Contributor

@snario snario commented May 6, 2021

Removes skipLibCheck, paths, and baseUrl from base typescript config. It's not clear from the commit history why these were added, and the integration tests have passed for me locally on a fresh build with these changes added.

@changeset-bot
Copy link

changeset-bot bot commented May 6, 2021

⚠️ No Changeset found

Latest commit: 5737ebd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@snario snario requested a review from smartcontracts May 6, 2021 23:40
smartcontracts
smartcontracts previously approved these changes May 7, 2021
@smartcontracts smartcontracts dismissed their stale review May 7, 2021 00:10

Requesting review by Georgios

@smartcontracts
Copy link
Contributor

Requesting that @gakonst take a look at this -- was this something you added?

@gakonst
Copy link
Contributor

gakonst commented May 7, 2021

this is useful so that your IDE/editor can have code navigation/completion features just by cloning the repo, without requiring you to build it.

"paths": {
      "@eth-optimism/*": ["packages/*/src", "integration-tests/*/test"]
    },

The baseUrl and skipLibCheck can go.

Comment on lines -5 to -7
"paths": {
"@eth-optimism/*": ["packages/*/src", "integration-tests/*/test"]
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep this. OK with removing the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This affects certain packages behaviour. For example, running tslint in packages/contracts will lead to linting of packages/smock I think because it interprets it as part of the source code. I haven't fully debugged that example in particular but removing this line fixes it.

Do we need that feature? I prefer building the packages and using TypeScript's Project References.

Copy link
Contributor

@gakonst gakonst May 7, 2021

Choose a reason for hiding this comment

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

KK - was thinking it's a better experience, but if it's a PITA np.

@gakonst gakonst merged commit 467e275 into master May 7, 2021
@gakonst gakonst deleted the liam/no-unneeded-tsconfig-args branch May 7, 2021 21:16
InoMurko pushed a commit to omgnetwork/optimism that referenced this pull request May 25, 2021
* style: fix indenting

* build: remove skipLibCheck, paths, and baseUrl from base typescript config
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