Workflow to suggest `[no important files changed]`

That worked! Merged: tracks: add "no important files changed" workflow by ErikSchierboom · Pull Request #322 · exercism/org-wide-files · GitHub ci: add "no important files changed" workflow (#322) · exercism/org-wide-files@42a0965 · GitHub will automatically create PRs for all track repos.

Just got the PR to the Python track. Don’t know if this is cause for alarm or not, but this particular PR came with the following message for me:

Should I merge, or hold off on merging?

It’s the little robot icon. Should be fine, but I’ll make sure to remove it from the code (if I can find it).

@BethanyG See Remove unicode in branch name by ErikSchierboom · Pull Request #326 · exercism/org-wide-files · GitHub

1 Like

So strange. It’s not like that :robot: wasn’t there in past PRs…wonder what changed with GitHub?

I guess someone abused it somehow

Question: how often will this run? How many bot comments will we get per PR?

Max 1 comment per PR. (at the time the PR is created)

I ran into a problem related to this.
In preparation for the exercise Acronym being featured on PowerShell for 48in24, I went over the exercise to write approaches and realize the test suite has a problem.
In an oversight, the old test suite doesn’t test for proper capitalization, so I went and updated the test suite and the example.

The PR is failing the newest check
Error: Unhandled error: HttpError: Resource not accessible by integration
just right under
url: 'https://api.github.com/repos/exercism/powershell/issues/343/comments',

Seem like it can’t access the comment section
@ErikSchierboom Could you take a look at this?

And if an important file is subsequently changed by a new commit?

If the diff doesn’t contain a change to an important file at the time the PR is created, there will be no reminder message. If a change to an important file is made later → no reminder.

We discussed this briefly, the problem is exactly that we wouldn’t want to generate too many messages. And extending the script to check every time if a reminder message exists already… seemed tedious and complicated for little benefit.

Ah, that’s due to a fork being used, which then messes with the permissions. I’ll fix next week

@glaxxie Would you mind creating a PR for PowerShell that has this change to the workflow: Add token to no-important-files-changed workflow (#1227) · exercism/fsharp@5fe63fb · GitHub

Once that is merged, could you then open another PR to see if that fixes it.

I got the PR up.

Just need a quick approval and then merge.
Edited: Merged. I will report back when I got a PR next to see if it is all good.

Also, Erik, for what it worth, despite failing the newest check and passed 4/5, it still allowed me to merged when I tried again yesterday

The merge button was a dark color instead of the usual blue though.

Great, thanks!

By default, only the configlet check is required, the other CI results can be ignored. I’m happy to make some of them required if you’d like.

Happy to report that the workflow now work flawlessly with a new PR!

By default, only the configlet check is required, the other CI results can be ignored. I’m happy to make some of them required if you’d like.

Actually this might work for my advantage. I have a PR for the Ledger exercise that has been parked since December. It passed every test, except the example run in ubuntu env (window envpassed with no problem.)
I have checked back and forth many times and I’m quite confident the solution in the example file is correct, however it might have failed in the ubuntu env because of the usage of cultureinfo from .NET.
If skipping the failed test is allow in this case, I would have you to take a look of it first in review and then merge later.

Cool. Then I’m gonna batch sync the change to all tracks

CultureInfo has always been problematic. I’m looking at the C# implementation and I could get it working somehow using:

private static CultureInfo CultureInfo(string locale)
{
    switch (locale)
    {
        case "en-US": return new CultureInfo("en-US");
        case "nl-NL": return new CultureInfo("nl-NL");
        default: throw new ArgumentException("Invalid locale");
    }
}

Did you try something like that? If not, the only thing that I could suggest is to use

- uses: actions/setup-dotnet@4d6c8fcf3c8f7a60068d26b594648e99df24cee3
  with:
    dotnet-version: "7.0.100"

Within the workflow that runs the tests.

In the Red repo, I’m seeing a recurring RequestError [HttpError]: Resource not accessible by integration.

Example at Sync run-length-encoding · exercism/red@436474d · GitHub

Seeing the same for Python: Mecha Munch Management - Fix misspelling of 'aisle' as 'isle' · exercism/python@1dd4a94 · GitHub