tomerfi / auto-me-bot Goto Github PK

What did you have in mind?

I think that using faker.js to generate names and emails is more standard in testing (I'm not really sure I got this by reading over the internet).

For example, this piece of test can use that.

    const fakeAuthorName = 'Elias Author';
    const fakeAuthorEmail = '[email protected]';
    const fakeCommitterName = 'Ezekiel Committer';
    const fakeCommitterEmail = '[email protected]';

Are you trying to fix a problem?

No response

Any lead on how this feature can be implemented?

No response

What happened?

Technically, this line identifies bots, but it comes with a design bug (or even more):

For the record, bots get a pass from signing off on their commits.

The isBot verification is being performed per handler request, and not per commit in the request.

As it is currently designed, for PRs with mixed commits, i.e. commits sourced by bots and commits sourced by humans, the isBot is based on the last commit.

An example I came across, a new PR was opened by dependabot, the pr-signed-commits workflow passed but some other workflow of mine had failed on account of the change brought forward by depenedabot's dependency update.
Once I resolved and pushed it to the PR, my other workflow was passing, but pr-signed-commits workflow was now constantly failing because the last commit was from my user, and I'm not a verified bot, so the handler expected dependeabot to signed off on its commits as well.

That being said, even though the handler is not supposed to expect bots to sign off on their commits, dependabot does!
So although cases like the example I described above are in fact a bug and shouldn't happen, when it does happen, for some reason (allegedly another bug), the handler is not able to verify dependabot's sign-off trailer.

Please provide runtime information.

Irrelevant.

Relevant log output

An example of a commit message by `depenendabot`, which the handler wasn't able to pick up on its sign-off trailer:

build(deps-dev): bump flake8 from 4.0.1 to 5.0.1
Bumps [flake8](https://github.com/pycqa/flake8) from 4.0.1 to 5.0.1.
- [Release notes](https://github.com/pycqa/flake8/releases)
- [Commits](PyCQA/[email protected])

---
updated-dependencies:
- dependency-name: flake8
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

How can this issue be reproduced?

• Wait for a PR by dependabot
• Verify the pr-signed-commits handler passes
• Add a commit of your own
• Verify the pr-signed-commits handler fails

What did you have in mind?

The behavior on win is weird, some of the fields in tests results duplicate for some reason.
'```\r\n' + 'chore: unit test this thing\n' + '\n' + 'chore: unit test this thing\r\n' + '\r\n' + 'Lorem ipsum dolor sit amet\r\n' + '```\r\n' +

Are you trying to fix a problem?

Any lead on how this feature can be implemented?

No response

What did you have in mind?

set-output is deprecated:
https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

We use it in .github/workflows/release.yml.

Are you trying to fix a problem?

Prepare for deprecation by GitHub.

Any lead on how this feature can be implemented?

Using environment files.

What did you have in mind?

What is the issue?

Nowadays everyone uses Prettier, An automated code formatting tool.

The issue is everyone have different global settings of prettier according to their style. BUT this can cause useless changes to opensource projects as many people works on same code.

So having a prettier config file inside project directory can resolve this issue.

Are you trying to fix a problem?

Yes.

Any lead on how this feature can be implemented?

How to solve this issue?

adding a file .prettierrc and .prettierignore will solve this issue

What did you have in mind?

Add the ability to configure the various handlers, to be more precise, the conventional commits handler screams a lot about commit message length, especially for dependabot's commits in repos containing their own sub repos/projects.

Example:

Are you trying to fix a problem?

No response

Any lead on how this feature can be implemented?

No response

What did you have in mind?

It seems like there is a demand to enforce conventional commits on PR titles as well.

Are you trying to fix a problem?

No response

Any lead on how this feature can be implemented?

@TomerFi suggested here on these 2 optional design.

---
pr:
  conventionalCommits:
    includePrTitle: true
    rules:
      'body-max-line-length': [2, 'always', 150]

---
pr:
  conventionalPrTitles:
  conventionalCommits:
    rules:
      'body-max-line-length': [2, 'always', 150]

I tend to agree with the second option. Because as the example shows it does not really make sense to custom rules the same way for both.

What happened?

I've been witnessing occasions where the pr-tasks-list handler returns a successful outcome although not all tasks were checked.

An example can be viewed in this PR, although the first task is unchecked, the run seems to be successful.

This is pretty much all I have so far, I haven't had the time to investigate this.
If I'll bump into more occurrences of this weird "bug", I'll report them back here.

Please provide runtime information.

Irrelevant.

Relevant log output

No response

How can this issue be reproduced?

No response

What did you have in mind?

Node 20 is the LTS version and was released about 10 months ago. See here.
Support was added by AWS about 3 months ago. See here.

See the previous bump for reference: issue #230 pr #279.

Are you trying to fix a problem?

No response

Any lead on how this feature can be implemented?

No response

What did you have in mind?

As suggested here #47 (comment), adding a feature to allow configuring the pr-signed-commits to ignore specific users or emails, sounds like a great idea.

Are you trying to fix a problem?

No response

Any lead on how this feature can be implemented?

Current .github/auto-me-bot.yml configuration for the pr-signed-commits handler is:

---
pr:
  signedCommits:

We can add option to ignore a list of users/email or perhaps both:

---
pr:
  signedCommits:
    ignore:
      users: ["dependabot[bot]", "anotheruser"]
      emails: ["[email protected]", "[email protected]"]

What did you have in mind?

Currently, all handlers have access to the full configuration file,
this is unwanted - the handlers don't need each other's configuration,
keeping it this way might invite coupling as this repo evolve and isn't the most secure thing to do.

Are you trying to fix a problem?

Prevent handlers from accessing each other's configuration.

Any lead on how this feature can be implemented?

As of the time of writing this, we have 3 handlers:

• pr-conventional-commits
• pr-signed-commits
• pr-tasks-list

This is the handlers' distribution line:

The handlers, for example, pr-conventional-commits, can access the following configuration:

pr:
  conventionalCommits:
    x:
      y: z

like so:
config.pr.conventionalCommits.x.y
and, of course, can access any other configuration that exists in the configuration file.

If we change the following line, passing only the relevant configuration:

invocations.push(handlers[key]()(context, config.pr[key], startedAt));

this should mean that for the example above, the pr-conventional-commits will access its configuration like so:
x.y
and will not have access to it's parent configuration.

What did you have in mind?

Bump to Node 18 had been the lts version for more than a year.
Node 16 (the previous lts) isn't under active support for 8 month and drops security support in 2.

See https://endoflife.date/nodejs

This will requires:

• update package.json
• update ci workflow
• prepare aws environment

Are you trying to fix a problem?

No response

Any lead on how this feature can be implemented?

No response

See https://github.com/orgs/community/discussions/107789 discussion for further info.
Issue opened in Probot: probot/probot#1974

What did you have in mind?

We use Python 3.10 to build the documentation site.

Are you trying to fix a problem?

No response

Any lead on how this feature can be implemented?

No response

What did you have in mind?

Lately, A lot of NodeJS packages have migrated from CommonJS to ESModules. Which are not supported in this project. The following PRs are blocked:

• #397
• #403
• #409
• #423
• #398
• #401
• #407
• #424
• #399
• #402
• #411

Also, see TomerFi/github-viewer-stats#74.

This project is not consumed as a module, so it is only affected by the modules it consumes. It can either be migrated to ESModule or switched from require statements to dynamic import ones.

Are you trying to fix a problem?

No response

Any lead on how this feature can be implemented?

No response