Skip to content

Conversation

@Arxcis
Copy link
Owner

@Arxcis Arxcis commented Dec 8, 2020

TLDR;

A lot of stuff is changing, but the repo will still be familiar to you 👍
image

Why?

  1. Make it possible for everyone to submit their own input, without worrying about getting conflicts with others.

  2. It is helpful to test against multiple distinct input/output-pairs, to make sure our solutions are robust - does not make assumptions about a specific input/output-files being provided.

arxcis.input
arxcis.output
preng.input
preng.output

Suggested change

Change the signature of the language-tests to take a list of SOLUTION_FILES and a list of IO_FILES as input:

Example declaration in languages/go.sh

#!/usr/bin/env bash
set -euo pipefail

# Usage:      ../../lang/go.sh  "SOLUTION_FILES"   "IO_FILES"
#
# Example:    ../../lang/go.sh  "solutions/*.go"   "io/*"
# Expands to: ../../lang/go.sh   solutions/main.go  io/alice.input io/alice.output io/bob.input io/bob.output
#
SOLUTION_FILES=$1   # Expand $1 into list
IO_FILES=$2         # Expand $2 into list

Example usage in days/day-05/test.sh

echo ""
echo "--- Day 5: Boarding pass ---"

$D/../../lang/go.sh     "$D/solutions/*.go"   "$D/io/*"
$D/../../lang/c.sh      "$D/solutions/*.c"    "$D/io/*"
$D/../../lang/sml.sh    "$D/solutions/*.sml"  "$D/io/*"
$D/../../lang/python.sh "$D/solutions/*.py"   "$D/io/*"
$D/../../lang/deno.sh   "$D/solutions/*.ts"   "$D/io/*"

echo ""

Example running ./days/day-05/test.sh

--- Day 5: Boarding pass ---
cat INPUT | go       day05.stektpotet.go     2ms        ✅
cat INPUT | go       tholok97.go             2ms        ✅
cat INPUT | gcc      day05.c                 2ms        ✅
cat INPUT | polyc    day05.sml               4ms        ✅
cat INPUT | python3  day05.klyve.py          15ms       ✅
cat INPUT | python3  day05.preng.py          11ms       ✅
cat INPUT | python3  day05.stektpotet.py     11ms       ✅
cat INPUT | python3  one-liner.stektpotet.py 12ms       ✅
cat INPUT | deno     day05.ts                15ms       ✅

@Arxcis Arxcis added the RFC label Dec 8, 2020
@Arxcis Arxcis requested review from Stektpotet and tholok97 December 8, 2020 15:30
@tholok97
Copy link
Collaborator

tholok97 commented Dec 8, 2020

I'll have a look after work @Arxcis :)

@Arxcis Arxcis force-pushed the multiple-tests-per-day branch 3 times, most recently from 2961930 to 53fc954 Compare December 8, 2020 17:32
@Arxcis Arxcis force-pushed the multiple-tests-per-day branch from 53fc954 to ed795ba Compare December 8, 2020 17:32
@Arxcis Arxcis force-pushed the multiple-tests-per-day branch from cca54c1 to e07e3bd Compare December 8, 2020 18:10
@tholok97
Copy link
Collaborator

tholok97 commented Dec 8, 2020

@Arxcis sorry I got hung up doing aoc with a friend over discord, and then I hit my head against the wall with lisp for a while.. I don't know when I'll have time to take a proper look at this. I have a suggestion, but I don't feel comfortable giving it until I've tried to look for myself how it will work in practice.

(Btw behold my creation:

image

A precursor to how I want to do day 1 in Lisp)

@Arxcis
Copy link
Owner Author

Arxcis commented Dec 8, 2020

@tholok97 No worries. I have the feeling, I am going to need a few days to settle on something myself. This PR is not currently building in the CI, so you can hold off reviewing, until I have at least figured that one out.

We can do the review/discussion/grooming in IRL if you like 😉 The good thing with this PR, is that we will eventually have something to look at while discussing .

@Arxcis Arxcis added the refactor label Dec 8, 2020
@Arxcis Arxcis changed the title Proof of concept - Testing against multiple input/output-pairs Proof of concept - Testing against multiple solutions and multiple input/output-pairs Dec 8, 2020
fix Conflicts:
	days/day-03/test.sh
	days/day-08/test.sh
@Arxcis Arxcis marked this pull request as ready for review December 8, 2020 23:08
@Arxcis Arxcis changed the title Proof of concept - Testing against multiple solutions and multiple input/output-pairs Testing against multiple solutions and multiple input/output-pairs Dec 8, 2020
@Arxcis
Copy link
Owner Author

Arxcis commented Dec 8, 2020

You know what. I got a breakthrough. The tests are passing, and I am happy with how the new architecture works. I am comfortable with setting this to "Ready for review" now, as I am going to bed 🛏️ 😴

@Arxcis
Copy link
Owner Author

Arxcis commented Dec 9, 2020

I got a really good review from @tholok97 IRL 😄 Thank you very much! 🙏 I will take all the things into consideration when making a decision on what to do tonight.

@Arxcis Arxcis mentioned this pull request Dec 9, 2020
@Arxcis Arxcis removed the RFC label Dec 9, 2020
fix Conflicts:
	days/day-09/input.txt
	days/day-09/output.txt
@Arxcis Arxcis force-pushed the multiple-tests-per-day branch from 7fc0afb to c3dceba Compare December 9, 2020 19:39
@Arxcis Arxcis merged commit b4edd34 into main Dec 9, 2020
@Arxcis Arxcis deleted the multiple-tests-per-day branch December 9, 2020 21:35
Copy link
Collaborator

@Stektpotet Stektpotet left a comment

Choose a reason for hiding this comment

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

From what I saw, only a couple of minor things need to/can be "fixed" here.
As a whole, nice merge 👍

```

7. When you are happy with local testing, make a Pull Request to the `main` branch.
8. One of the maintainers will merge PR's, at the of each day.
Copy link
Collaborator

Choose a reason for hiding this comment

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

One of the maintainers will merge PR's, at the ................. of each day.

Copy link
Owner Author

Choose a reason for hiding this comment

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

👀 Well spotted. Since I already merged, I will commit a fix for this directly 👍

Comment on lines 16 to 17
# $DAY
## --- Part 1 ---
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm pretty sure most of the readme fiiles we have so far have these two combined.

It could be changed to this:

# --- $DAY: Title ---

Not that that really matters in the template, as it's going to be written over at some point again...
¯\_(ツ)_/¯

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think it matters, even if it is a minor thing, because the template is supposed to help the developer. It is a way of documenting things, and communicating intent, so that any contributor is capable of contributing README.md's. I will fix in a commit directly. 👍

@Arxcis
Copy link
Owner Author

Arxcis commented Dec 10, 2020

Thank you for reviewing @Stektpotet 🙏

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.

4 participants