Skip to content

Conversation

Zearin
Copy link
Contributor

@Zearin Zearin commented Jan 22, 2022

Addresses #5139, #5140, and #5427.

Changes:

Summary:

  • This commit converts all the uses of @alt I found in 📁src/color.
  • Since @alt was not linted for line length while describe(…) is, I had to add line breaks to the descriptions for the lint to pass (and allow the commit to go through).

Related Issues & Pull Requests:

PR Checklist

**Summary:**
- This commit converts all the uses of `@alt` I found in 📁`src/color`.
- Since `@alt` was not linted for line length while `describe(…)` is, I had to add line breaks to the descriptions for the lint to pass (and allow the commit to go through).

**Related Issues & Pull Requests:**

- This is a small piece of the checklist described in Issue processing#5139 (by @lm-n).
- That issue, in turn, was begun in Pull Request processing#5140 (also by @lm-n).
- In Issue processing#5427, the idea requiring this for the inline docs got positive support from several members (including @lmccart, @outofambit, and @catarak).
@Zearin
Copy link
Contributor Author

Zearin commented Jan 23, 2022

Weird…I’ve not encountered a change to the docs that had any impact on the tests before!

There are 8 failing tests here, and all appear to be the same error. Here is the first error for convenience:

1) src/color/p5.Color.js
2063
       setRed documentation
2064
         example #1 works:
2065
     TypeError: Cannot read properties of undefined (reading 'id')
2066
      at p5._main.default.describe (http://localhost:9001/lib/p5.js:53592:37)
2067
      at eval (eval at <anonymous> (test-reference.html:36:11), <anonymous>:11:1)
2068
      at http://localhost:9001/test/test-reference.html:68:11
2069
      at new p5 (http://localhost:9001/lib/p5.js:64318:17)
2070
      at http://localhost:9001/test/test-reference.html:146:20
2071
      at new Promise (<anonymous>)
2072
      at Context.testFunc (test-reference.html:139:18)
2073

Unfortunately, I have no idea how to interpret this error message, or what to do about it.

Can a maintainer or Processing member help point me in the right direction?

@limzykenneth
Copy link
Member

You might have already done so but I would prefer if you can get in touch with @lm-n before starting work on this issue, simply in case there are duplicate work already done.

The tests includes running all the example sketches in the inline reference which is where the error likely comes from. @lm-n would know more about this issue than I do which is why I would advise getting in contact first.

@Zearin
Copy link
Contributor Author

Zearin commented Jan 29, 2022

@lm-n: I would love to help coordinate with #5139 and #5140. Are you available?

@Zearin
Copy link
Contributor Author

Zearin commented Jan 30, 2022

@limzykenneth : According to @lm-n’s GitHub Profile page, he hasn’t been active in several months.

Assuming it’s not possible to coordinate with @lm-n, how do you suggest I should proceed?

@lm-n
Copy link
Member

lm-n commented Jan 30, 2022

@Zearin I've reviewed your changes and will look at the errors in a bit.

@Zearin
Copy link
Contributor Author

Zearin commented Jan 30, 2022

I stand corrected. 😄 Thank you for the guidance, @lm-n !

Copy link
Member

@lm-n lm-n left a comment

Choose a reason for hiding this comment

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

@Zearin describe() must be inside the setup() or draw() that's where the error is coming from!

@lm-n
Copy link
Member

lm-n commented Jan 31, 2022

@Zearin & @limzykenneth I'm merging this PR! Thanks @Zearin, feel free to help transition more reference example to use describe()!

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.

3 participants