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

fix: several typos in concepts #548

Merged
merged 3 commits into from
Aug 26, 2023
Merged

Conversation

vnkmpf
Copy link
Contributor

@vnkmpf vnkmpf commented Aug 24, 2023

No description provided.

@neenjaw
Copy link
Collaborator

neenjaw commented Aug 24, 2023

you need to fix also the underlying concepts which are used in the template generation. you can find them in /concepts

@neenjaw neenjaw added x:size/tiny Tiny amount of work x:rep/tiny Tiny amount of reputation labels Aug 24, 2023
Copy link
Collaborator

@neenjaw neenjaw left a comment

Choose a reason for hiding this comment

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

see comment

@vnkmpf
Copy link
Contributor Author

vnkmpf commented Aug 24, 2023

Thanks for the comment.
I fixed some more typos in the directory.

There is still the mysterious failure of one check, even though the other PR worked 🤷

@neenjaw
Copy link
Collaborator

neenjaw commented Aug 24, 2023

That CI failing is because that step generates a difference between the hydrated template and the resulting introduction document. So when it tries to commit the difference in the branch, it fails since it doesn't have permission to commit to your branch (nothing for you to fix there). If you pull this locally then run from the repository root directory:

./bin/fetch_configlet

then

./bin/configlet generate

you will be able to see the difference generated. So next time it might be easier to have this workflow:

  1. Make a new branch locally
  2. Make the changes to the introduction/about files in /concepts
  3. run config generate
  4. Then make the changes in the instructions and hints since they are not auto-generated.

(Side note, @ee7 can we add a comment to files computed by configlet generate that they are auto-generated and not to be manually edited? It might ease this process next time someone is onboarding)

@ee7
Copy link
Member

ee7 commented Aug 25, 2023

(Side note, @ee7 can we add a comment to files computed by configlet generate that they are auto-generated and not to be manually edited? It might ease this process next time someone is onboarding)

Yeah, we could do that. It's tracked in exercism/configlet#655.

But it'll also help when configlet generate has the same design (planned in exercism/configlet#619) as fmt and sync, such that writing changes requires an extra flag. Then we can consider enabling configlet generate in every track CI via the configlet workfow, and recommend running generate if the repo files are not up to date.

@vnkmpf
Copy link
Contributor Author

vnkmpf commented Aug 26, 2023

@neejaw Thank you for posting those steps. I thought the problem was the same as before.
Now that I followed the steps, it seems to be fixed.

@vnkmpf vnkmpf requested a review from neenjaw August 26, 2023 16:52
Copy link
Collaborator

@neenjaw neenjaw left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! Yes, maybe we could add those suggestion to that workflow when it fails to be a bit more helpful when this situation happens. Do you want to look into that process with GitHub actions?

@neenjaw neenjaw merged commit 2deb482 into exercism:main Aug 26, 2023
20 checks passed
@vnkmpf
Copy link
Contributor Author

vnkmpf commented Aug 26, 2023

I have zero experience with GitHub Actions, so it would be probably more of exploring and getting familiar with than help.
If it's something that is desired sooner than later, I think I cannot help much.
But if it's a nice-to-have, I might take a look :)

@neenjaw
Copy link
Collaborator

neenjaw commented Aug 28, 2023

It's just a nice to have, no real rush on it. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:rep/tiny Tiny amount of reputation x:size/tiny Tiny amount of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants