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

⚗️ ci: run solutions test for file change in related project #63

Merged

Conversation

fuadop
Copy link
Contributor

@fuadop fuadop commented May 31, 2022

PR Checklist

Overview

Runs test:solutions script for folder of changed files.

@fuadop fuadop changed the title ⚗️ ci: run solutions test for file change in related project ⚗️ ci: run solutions test for file change in related project May 31, 2022
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

This is a very good start 🎉 ! I hadn't seen the tj-actions/changed-files action before, thanks for teaching me about it. It looks like a perfect fit for what this PR needs 😄.

Requesting changes on a bit of code cleanup and also -unless you think this is a bad idea?- augmenting solutions.yml instead of having two workflows.

.github/workflows/changed_solutions.yml Outdated Show resolved Hide resolved
.github/workflows/changed_solutions.yml Outdated Show resolved Hide resolved
.github/workflows/changed_solutions.yml Outdated Show resolved Hide resolved
.github/workflows/changed_solutions.yml Outdated Show resolved Hide resolved
.github/workflows/changed_solutions.yml Outdated Show resolved Hide resolved
.github/workflows/changed_solutions.yml Outdated Show resolved Hide resolved
.github/workflows/changed_solutions.yml Outdated Show resolved Hide resolved
.github/workflows/changed_solutions.yml Outdated Show resolved Hide resolved
@fuadop
Copy link
Contributor Author

fuadop commented Jun 2, 2022

This is a very good start tada ! I hadn't seen the tj-actions/changed-files action before, thanks for teaching me about it. It looks like a perfect fit for what this PR needs smile.

Requesting changes on a bit of code cleanup and also -unless you think this is a bad idea?- augmenting solutions.yml instead of having two workflows.

I also just found out about the changed-files action 😅 . There are a few out there for this changed files. I saw about 3 but this one had the most starsss.

@fuadop
Copy link
Contributor Author

fuadop commented Jun 2, 2022

Yea, I love the idea of augumenting it to one workflow ! It's brilliant ! @JoshuaKGoldberg

@fuadop
Copy link
Contributor Author

fuadop commented Jun 4, 2022

Hey @JoshuaKGoldberg , I have made changes 😇 . How do you see this ? 😁 I think it looks better now . You're brilliant ! ✨
Thanks for this !

Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Looks like we missed a chmod +x, heh - my bad. This looks pretty great so far otherwise!

It's hard to test this kind of thing locally though. Could you add a couple of commits that make some changes to project tests? At least one change that causes a failure, and at least one change that doesn't. That'll show that this PR works like it seems it does.

By the way, I'm going to be in and out of availability the next few days. You don't have to @ ping me - I've set notifications up so I'll see anything you push here. Feel free to re-request review with in the GitHub UI when this is ready.

Thanks! 🙌

.github/workflows/solutions.yml Outdated Show resolved Hide resolved
@fuadop
Copy link
Contributor Author

fuadop commented Jun 5, 2022

hmm 🤔 , Looks like it's bent on the first few commits.

@fuadop
Copy link
Contributor Author

fuadop commented Jun 7, 2022

Yesssss, I've done ittt.

@fuadop
Copy link
Contributor Author

fuadop commented Jun 7, 2022

@JoshuaKGoldberg
Copy link
Contributor

Agh sorry this dropped off my radar - reviewing now!

If I ever take more than a week feel free to re-request review in the GitHub UI. Or if that button's not available ping me. It's definitely that I'm missing things, not that I'm ignoring you!

@JoshuaKGoldberg JoshuaKGoldberg self-requested a review June 18, 2022 19:51
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Amazing! Thanks so much @fuadop, this is really fantastic 🔥. CI was getting really slow until now. And now it won't be!

@JoshuaKGoldberg JoshuaKGoldberg merged commit 49d49c0 into LearningTypeScript:main Jun 18, 2022
@fuadop
Copy link
Contributor Author

fuadop commented Jun 19, 2022

If I ever take more than a week feel free to re-request review in the GitHub UI. Or if that button's not available ping me. It's definitely that I'm missing things, not that I'm ignoring you!

Sure!

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.

🛠 Tooling: Run only changed solution scripts in CI
2 participants