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

Musketeer variant #762

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

alperatalayn
Copy link

No description provided.

@ianfab
Copy link
Member

ianfab commented Mar 12, 2024

Thank you. Since there already are several other open PRs review will likely take some time. Could you please in the meantime try to fix the errors/warnings reported by CI?

@ianfab
Copy link
Member

ianfab commented Apr 4, 2024

@alperatalayn @cross-of-north @sayantandey @chris-heldeis Thanks for your PRs. May I ask you to align on your PRs and come up with a single PR? I have fairly limited time for reviews, so reviewing 4 versions of the same PR is not really practicable for me. Would be very nice if you get to a consensus on which is the most refined version that should be reviewed.

@cross-of-north
Copy link

I consider my version to be the most refined.

  • @alperatalayn initial version introduces basic structures to support musketeer variant , but breaks build and tests. Also it does not work.

  • @chris-heldeis version fixes some problems, but some tests still fail (at least it was so last week, and new commits don't contain anything relevant). Also Python and JS binding versions are incremented; I can't say if it is useful or not.

  • @sayantandey version: may be I'm not aware of some Github features, but in my opinion this version does not contain anything except @alperatalayn commits, so it's effectively empty.

  • @cross-of-north (mine) version - build and tests are all green, contains improvements/fixes to make the variant actually playable and compatible with GUI (Winboard) and with implementations in other engines.

@sayantandey
Copy link

sayantandey commented Apr 5, 2024

I consider my version to be the most refined.

  • @alperatalayn initial version introduces basic structures to support musketeer variant , but breaks build and tests. Also it does not work.
    ...
  • @sayantandey version: may be I'm not aware of some Github features, but in my opinion this version does not contain anything except @alperatalayn commits, so it's effectively empty.

My PR (as I mentioned in the name 'Fixes For#762 Pipeline fails') was to fix the build issues in the PR pipelnie from @alperatalayn version and make sure the windows distributables are successfully built and being able to run, I created the build to check the tests in PR pipeline (the make builds are already tested in my local) after fixing those issue. So, yes you can ignore my PR considering this.

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.

4 participants