Skip to content
This repository was archived by the owner on Jun 7, 2023. It is now read-only.

Conversation

@ascholerChemeketa
Copy link
Contributor

  • Moved Pyflakes code coach to its own file.
  • Update coach run/render code to more cleanly support multiple coaches.
  • Set up Code Coach string for internationalization.
  • Fix bug where coach div not removed if all issues fixed

Coaches are added to a list during ActiveCode construction.

runCoaches function is triggered on program run. It asynchronously runs all coaches, but then waits for all results so Code Coach div can display any coach feedback deterministically in the order coaches were created.

Questions/Comments:

  • I could definitely see a case for more flexible addition of coaches to ActiveCodes instead of hard coding based on language in the constructor. I just don't have a clear idea of what that should look like.
  • I think the internationalization is right...

@ascholerChemeketa
Copy link
Contributor Author

I stress-tested the deterministic output with a TestCoach that just waited a random amount of time before returning a set string. Works as intended when there are multiple coaches taking variable amounts of time.

@ascholerChemeketa
Copy link
Contributor Author

One aspect that might be annoying in existing books - Pyflakes complains about * imports.

Personally, I am using this as an excuse to replace them all in my book. But it would not be too hard to filter them out in the Pyflakes coach.

image

let code = await this.buildProg(false);
let results = [];
for(let coach of this.codeCoachList) {
results.push(coach.check(code));
Copy link
Member

Choose a reason for hiding this comment

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

Should there be a method to add a coach? If I'm thinking about the future and adding a new coach I would want to have a method to do that so I don't have to modify this code...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No one should have to touch that code.

Coaches are added up in the constructor (line 143). I'm not sure where else they would be injected from - that was one of my questions. I don't have a good sense of how to make creating coaches more modular.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hrm, can't figure out how to quote in those lines without starting a new conversation.

Copy link
Member

Choose a reason for hiding this comment

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

I was imagining a method on the Activecode instance, but then I'm not sure what other object would call it, so having it in the constructor is fine for now.

@ascholerChemeketa ascholerChemeketa marked this pull request as draft April 7, 2023 16:00
@ascholerChemeketa
Copy link
Contributor Author

Updated to filter import * related warnings from pyflakes to avoid scary warnings in existing books.

@ascholerChemeketa ascholerChemeketa marked this pull request as ready for review April 7, 2023 20:47
@bnmnetp bnmnetp merged commit dab6a11 into RunestoneInteractive:master Apr 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants