Meeting minutes
Minutes Review
<dape> Oct-23
Daniel: I quickly checked the minutes
… we had a very short call last time
… we mostly discussed integrating PRs that were made for the publication branch
… I looked into it and prepared a PR for that
… other than that, the minutes look to me
… if there are no objections, then I would ask Kaz to publish
Kaz: The call was very short indeed and we did not approve the minutes of the meeting before
… so we need to do that as well
<kaz> Oct-2
Daniel: (adds the minutes from Obtober 2nd to the Wiki)
… (shows the minutes from that meeting)
… I think they are fine as well, so we can make both minutes public
Minutes are approved
PRs
Daniel: I would skip the TS-related PRs, since Cristiano is not here
… and the first PR deals with the adjustment of WoT definitions which might require more discussion
PR 511
<dape> PR 511 - fix: "Intel" usage to "Intel Corp."
Daniel: This is a very simple PR that came from other specs as well
… is regarding Zoltan's affiliation
… requested by Michael McCool, changes "Intel" to "Intel Corp."
… are you fine with the changes, Zoltan?
Zoltan: The changes are okay for me
Daniel: Then I will merge
PR is merged
PR 513
Daniel: This is one is probably a bit more tricky
… aligns TypeScript definitions with discovery API
<dape> PR 513 - feat!(typescript): align TypeScript definitions with Discovery API
Daniel: changes look fine to me
Jan: PR simply aligns TS definitions with specification
… we had some discussion regarding constructors in interfaces
Daniel: We should maybe postpone adding a constructor to the ThingDiscoveryProcess interface
Zoltan: We should resolve the constructor discussion first
Daniel: Adding constructors to interfaces was a bit new to me
Zoltan: Constructors are nice for testing, but we should not add them if we don't have a strong use case
… in general, we should use factories instead
… we should investigate why we decided to add constructors in the first place
… we already have factories, which is a simple enough approach
Daniel: Then I would ask you to update the PR, Jan, and remove the constructor
… one thing we need to do is changing the version number of the TypeScript definitions
Zoltan: Just to clarify: If someone creates a new ConsumedThing via a constructor, an error should be thrown, we should add a test case for that
Daniel: I think in node-wot, we don't even have a constructor defined
Jan: I think we don't have it in the TS definitions, but in the spec for ExposedThing
Daniel: A constructor is defined for ConsumedThing in the spec
… I think we have a consensus to remove it
… (adds a comment to PR 513 with the result of the discussion)
Zoltan: We also need to adjust the prose as well
Daniel: This is something we should do in a separate PR
… the only thing we should do is adjusting the ThingDiscoveryProcess and bumping the version
… will review again later and approve once changes are made
Zoltan: We need an issue to summarize the points
Daniel: There was an oversight of us not adjusting the TypeScript definitions to the prose
… specification was correct, only TypeScript definitions weren't correct
Zoltan: Is it possible to reference the PR where the changes to the specification were made?
Daniel: That is a good point
… there is no reference to the PR yet, the description should be updated
Jan: Will do
PR 515
<kaz> PR 515 - feat(typescript): add constructor to ConsumedThing interface
Daniel: Related to the PR before
… similar change to the constructor
Jan: This is actually obsolete after the discussion, 517 has been opened as an alternative
Daniel: (Adds a comment and closes 515 in favor of 517)
PR 517
<kaz> PR 517 - feat!: remove constructor from ConsumedThing interface
Daniel: This PR now only adjusts the specification text
… also still need to review it, adjusts prose and WebIDL
Zoltan: "Construct" is replaced with "create"
… prose is mostly kept the same
… I think this looks pretty good now
Daniel: Do you want to make any further reviews?
Zoltan: We should all have another look
… then we can probably decide next week
… let's keep it open for now then
PR 514
<dape> PR 514 - refactor: Update working draft with changes done for note2 publication
Daniel: This is a very simple PR, I guess
… in issue 497, we noticed that Kaz made some changes to the publication documents
… I essentially took out of them was is necessary to adjust in general
… there was one PR that added the "defer" attribute to the script tag loading ReSpec
… other than that, the GitHub API URL has changed
… so mostly aligns with published documents
… Cristiano approved it
Zoltan: You can just merge it
Daniel: (adds a comment and merges the PR)
Merged, issue 497 is automatically closed by the PR
Issues
Daniel: I haven't selected dedicated issues yet
… is there anything you would like to discuss?
… I haven't had time to look and group them
Issue 512
<kaz> Issue 512 - Add perl script (for fixing duplicate IDs) to master/release folder
Daniel: There is this issue that deals with the script by Kaz that removes duplicated IDs
… resolves the issue with the conformance classes
… from my point of view, keeping the conformance classes makes sense from a developer's perspective
… do you have any opinions on that?
Zoltan: I am also in favor of keeping the conformance classes
Daniel: That is good to hear, then we could simply close the PR
Zoltan: This was also something mentioned by McCool, having conformance classes gets rid of the need for multiple documents
Kaz: I am okay with adding the script in general, but we should have an updated README.md that details how to fix the ReSpec issues in general
Daniel: That's correct, if we add the script, then we should document how to use it
… do you have time to create a simple PR to add a short documentation text how to use it?
Kaz: There is a Makefile that documents how to use it, maybe we can add a one-line description based on that Makefile
Daniel: There were two PRs I've seen, one adds the script and one adds the Makefile
<kaz> Perl script
<kaz> Makefile which shows how to use the script
Kaz: That can simply be copied to or linked from the README
Daniel: Do we want to put it in the "Releases" folder or the main README?
… I think it is more related to Releases, but let me know what you want to see
… we are missing a README in the releases folder anyway
… so my suggestion is to add this README in there
Zoltan: But we have a README
Daniel: There are only READMEs for specific releases, not in the general directory
Kaz: Would make sense to add a README under /releases
… that README should include instructions how to use the script
… and how to create the static HTML files
<kaz> wot-scripting-api/releases
Kaz: we can use the README.md from the TD repo as a template, and can add how to use this Perl script to that.
<kaz> wot-thing-description/publication/README.md
Daniel: I will do that
… is that okay for everyone?
Zoltan: It's okay
… and have a README in every directory
Daniel: Is it then okay to close the discussion on conformance classes? And the corresponding PR?
<kaz> PR 502 - refactor: move WoT definitions up in a single place
<kaz> Issue 506 - Re-evaluate the conformance classes
Zoltan: Yes
Daniel: (closes PR 502 and issue 506, adds a comment to PR 502 with a summary)
Issue 516
Daniel: This issue can also closed, since we concluded in the discussion that we don't use constructors, neither in WebIDL nor in TypeScript
… since they are synchronous, while factories are asynchronous and can be used instead
… is it fine for everyone to close this issue?
Jan: Closing it is fine by me
Daniel: (Closes the issue)
AOB
Daniel: I would like to close early, any other business?
There is no other business
[adjourned]