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

export public api / types used by installLogsPrinter / installLogsCollector #128

Merged
merged 3 commits into from
Nov 18, 2021

Conversation

tebeco
Copy link
Contributor

@tebeco tebeco commented Nov 17, 2021

As installLogsPrinter and installLogsCollector are exported, then the inner types should also be exported:

installLogsPrinter.d.ts:

  • PluginOptions
  • Severity
    installLogsCollector.d.ts:
  • SupportOptions
  • Severity

Funny note:
Severity is redundant and might be moved to a common place some day when the repo will be migrated to typescript, also the typpings would be emitted by tsc so they would not need any manual maintainance

@tebeco tebeco changed the title export public api / types used by installLogsCollector export public api / types used by installLogsPrinter / installLogsCollector Nov 17, 2021
@tebeco
Copy link
Contributor Author

tebeco commented Nov 17, 2021

@archfz if you have any leads on how changing a .d.ts would break ~6 or more test or how to fix it, I'm all hear

@archfz
Copy link
Owner

archfz commented Nov 18, 2021

Looks good. I will retry the job. It most likely is flakiness.

@archfz
Copy link
Owner

archfz commented Nov 18, 2021

I have pushed some flakiness fixes for this test into develop. Please rebase and retarget your PR against develop.

I would also prefer not to do a release only with this change. Could you maybe take a look into #127 as well? Would appreciate some fresh eyes.

@tebeco
Copy link
Contributor Author

tebeco commented Nov 18, 2021

I wish github PR UI had a simple button to rebase.
Now i'm forced to clone my own repo 🙃 was doing that from the portal

…he inner types should also be exported

In order to properly be able to use an exported function, exporting the innertype would be more than usefull, as they are also part of the public API
@tebeco
Copy link
Contributor Author

tebeco commented Nov 18, 2021

@archfz, that seems to have fixed it ;)

@archfz archfz merged commit 262346b into archfz:master Nov 18, 2021
@tebeco tebeco deleted the patch-1 branch November 18, 2021 20:11
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