There are a lot of PR rejected about open-pixel-art

[albertsundjaja]

Actually, in my case it was because I forgot to update my repo with the latest code before submitting a pull request. So you should try fetching from the upstream git fetch upstream to pull the latest update

step to do it:

1. add the upstream
   git remote add upstream https://github.com/twilio-labs/open-pixel-art
git fetch upstream

2. pull it to your master branch
   git pull upstream master

3. merge your add-my-new-pixel branch
   git checkout master
git merge add-my-new-pixel

4. push it
   git push

5. then create the pull request, it should pass (just like what I did)

[dkundel]

Yeah I think we can fix that one by setting up a second GitHub action that runs on push events that will:

1. checkout
2. run npm install
3. run npm run prettier
4. run node tasks/sortPixels.js
5. run commit any changes using https://github.com/marketplace/actions/push-new-files-back-to-master

That way hopefully nobody should catch accidental formatting in their PRs.

[welcome]

Thank you so much for opening your first issue for this project! We'll try to get back to it as quickly as possible. While you are waiting...here's a random picture of a corgi (powered by dog.ceo)

[clanghout]

Due to the commit hook, the list gets automatically sorted, and master has some pixels that are not in place.
This means a lot of new pull requests touch more code than 1 line in the pixels data file, making the code review check fail on the danger step. A manually checked pr has to fix the ordering to allow these PRs to be accepted.

[dkundel]

Yeah there's a few different problems in here some of which are easier to fix than others. Updating your fork from upstream is the easiest way to fix some of the issues.

Git seems to sometimes randomly struggle with merging the changes in the pixels.json occasionally. The general strategy would be to accept current and then add your contribution back.

I'm open for any ideas to improve the bots.

[linda-lai]

Along with the upstream fetch problem, the docs said to run npm test - when I did, the linter also corrected some extra whitespace in the test suite, which then flagged that I had altered more than 1 file/a file other than pixels.json.

I ended up closing the PR and raising a new one. When I tried to merge that one, I noticed that it automatically ran the tests, passed and auto-merged. So I think this is also causing a lot of PR auto-merges to fail.

[ItaloSa]

@dkundel do you think that a pre-push hook solves this problem? We could use husky and configure these steps.

[dkundel]

Yeah I think so. I'm already creating a sync script that we could use that in a pre-push hook. Once #991 is done, this would be a great addition