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

feat: add types #35

Merged
merged 3 commits into from
Jan 7, 2022
Merged

feat: add types #35

merged 3 commits into from
Jan 7, 2022

Conversation

jamacku
Copy link
Contributor

@jamacku jamacku commented Dec 30, 2021

This should resolve #34

Please note, that this PR doesn't cover ProbotExports:

const ProbotExports = require("probot");
const pino = require("pino");
const { transport } = require("./pino-transport-github-actions");
module.exports = { ...ProbotExports, run };

@jamacku jamacku marked this pull request as draft December 30, 2021 16:00
@jamacku jamacku marked this pull request as ready for review December 31, 2021 07:46
index.d.ts Outdated
@@ -0,0 +1,7 @@
import { ApplicationFunction } from "probot/lib/types";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should work

Suggested change
import { ApplicationFunction } from "probot/lib/types";
import { ApplicationFunction } from "probot";

Can you check it please?

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 tried that at first, but my IDE was saying: Module '"probot"' has no exported member 'ApplicationFunction'.ts(2305) And When I checked probot's index.d.ts there is only export for ApplicationFunctionOptions but no ApplicationFunction type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll create PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Already merged, thanks! Now you'll have to bump the minimum version of probot to ^12.1.4

"probot": "^11.0.6",

Note there were some breaking changes in v12, I'm not sure if any of them affect this adapter:
https://github.com/probot/probot/releases/tag/v12.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, it seems like these breaking changes doesn't affect adapter, but I found out that there is failing one unit test.

It fails here:

expect(output[2].startsWith('::error::{"error":"nope"}%0A{%0A')).toBe(true);

With message:

expect(received).toBe(expected) // Object.is equality

    Expected: true
    Received: false

       97 |     process.stdout.write = origWrite;
       98 |
    >  99 |     expect(output[2].startsWith('::error::{"error":"nope"}%0A{%0A')).toBe(true);
          |                                                                      ^
      100 |
      101 |     expect(mock.activeMocks()).toStrictEqual([]);
      102 |   });

      at Object.<anonymous> (test/index.test.js:99:70)

Is it possible that error message format had changed or am I missing something?

@gr2m gr2m merged commit ba0dc7a into probot:master Jan 7, 2022
@gr2m
Copy link
Contributor

gr2m commented Jan 7, 2022

Děkuju Jan!

@github-actions
Copy link

github-actions bot commented Jan 7, 2022

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jamacku
Copy link
Contributor Author

jamacku commented Jan 8, 2022

Thank you for your work and this awesome tool! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFE: Please add support for TypeScript
2 participants