Skip to content

Component proper setter/getter, constructor may set its author, normalize optional version - #247

Merged
jkowalleck merged 7 commits into
masterfrom
component_author-issue246
Feb 2, 2022
Merged

Component proper setter/getter, constructor may set its author, normalize optional version#247
jkowalleck merged 7 commits into
masterfrom
component_author-issue246

Conversation

@jkowalleck

@jkowalleck jkowalleck commented Jan 26, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

@jkowalleck
jkowalleck requested a review from a team as a code owner January 26, 2022 17:53
@jkowalleck jkowalleck mentioned this pull request Jan 26, 2022
Closed
@jkowalleck
jkowalleck force-pushed the component_author-issue246 branch from 7a69ccd to 4a9efaa Compare January 26, 2022 17:56
@shiraze

shiraze commented Jan 26, 2022

Copy link
Copy Markdown

I think code should follow existing pattern, so:

this._author = (pkg.author?.name) ? pkg.author.name : undefined

And the expectation is that package.json would contain

...
 "author": { "name": "a name" },
...

Having said that, though, your code change would also be correct, but wouldn't it be better to use the "private" _author field?

@jkowalleck

jkowalleck commented Jan 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

re #247 (comment)
i thing the current code base is a mess and i dont want to continue like this anymore.
current code has proper setters for everything, and they do proper type checking - which is required to produce the proper XML/JSON results.

WHY on earth would one use the internal properties that are not type safe at all, if there are proper setters with type checks?

no, in don't think i will use the internal properties. i'll use the existing setters.

but i might use the optional-property style you suggested, @shiraze . thanks for the hint 👍

shiraze
shiraze suggested changes Jan 27, 2022
View reviewed changes

@shiraze shiraze left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As discussed in conversation set using optional property style, and we're all good :)

@jkowalleck
jkowalleck marked this pull request as draft January 27, 2022 20:52
@jkowalleck
jkowalleck force-pushed the component_author-issue246 branch from 670f7f4 to 73e7c61 Compare January 27, 2022 21:38
@jkowalleck

jkowalleck commented Jan 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

na, cannot use shorthand ala pkg.author?.name - it is not supported by node12 - which is the lowest supported version of this module.
see https://github.com/CycloneDX/cyclonedx-node-module/runs/4972957766?check_suite_focus=true

@jkowalleck
jkowalleck force-pushed the component_author-issue246 branch 4 times, most recently from 21b5f45 to 986b85b Compare January 27, 2022 21:56
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 27, 2022
View reviewed changes
Comment thread model/Component.js
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 27, 2022
View reviewed changes
Comment thread model/Component.js
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 27, 2022
View reviewed changes
Comment thread model/Component.js
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 27, 2022
View reviewed changes
Comment thread model/Component.js
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 27, 2022
View reviewed changes
Comment thread model/Component.js
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 27, 2022
View reviewed changes
Comment thread model/Component.js
@jkowalleck jkowalleck changed the title Component constructor may set its author Component constructor may set its author, defaults version Jan 28, 2022
@jkowalleck
jkowalleck force-pushed the component_author-issue246 branch 2 times, most recently from 1bf0c88 to 3cc38d0 Compare January 28, 2022 07:54
@jkowalleck jkowalleck changed the title Component constructor may set its author, defaults version Component proper setter/getter, constructor may set its author, normalize optional version Jan 28, 2022
@jkowalleck
jkowalleck force-pushed the component_author-issue246 branch from 3cc38d0 to e47d7e7 Compare January 28, 2022 07:59
@jkowalleck
Component rework
38c57ed 
detect component.author - fixes #246
normalize empty component.version - fixes #248
add JSDoc - #220
use private properties - #233

Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
use @babel/eslint-parser
41c5387 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
update locked dependencies and lockfile format
5603c70 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
jkowalleck force-pushed the component_author-issue246 branch from e47d7e7 to 5603c70 Compare January 29, 2022 11:36
@jkowalleck jkowalleck added CI Continuous Integration documentation labels Jan 29, 2022
@jkowalleck
jkowalleck force-pushed the component_author-issue246 branch 4 times, most recently from 148f9f4 to faf5366 Compare January 29, 2022 20:57
@jkowalleck
jkowalleck marked this pull request as ready for review January 29, 2022 20:59
@jkowalleck
jkowalleck force-pushed the component_author-issue246 branch from faf5366 to d07b8e3 Compare January 29, 2022 21:14
@jkowalleck
CT/CI generates jsdoc & play with jsdoc
fe6ccfd 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
jkowalleck force-pushed the component_author-issue246 branch from d07b8e3 to fe6ccfd Compare January 29, 2022 21:24
This was referenced Jan 29, 2022
Closed
Closed
@jkowalleck
wrote history
e5ce5c0 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck jkowalleck changed the title Component proper setter/getter, constructor may set its author, normalize optional version Component proper setter/getter, constructor may set its author, normalize optional version Jan 29, 2022
@jkowalleck
slacked components getterusage internally
02534e4 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 30, 2022
View reviewed changes
Comment thread model/Component.js
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 30, 2022
View reviewed changes
Comment thread model/Component.js
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 30, 2022
View reviewed changes
Comment thread model/Component.js
sonatype-lift[bot]
sonatype-lift Bot reviewed Jan 30, 2022
View reviewed changes
Comment thread model/Component.js
@jkowalleck
fixed JSDoc object annotation
17b43c4 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
brent-spiner
brent-spiner reviewed Feb 1, 2022
View reviewed changes
Comment thread .github/workflows/nodejs.yml
brent-spiner
brent-spiner reviewed Feb 1, 2022
View reviewed changes
Comment thread package.json
@jkowalleck

jkowalleck commented Feb 1, 2022

Copy link
Copy Markdown
Member Author

@brent-spiner made my day

@jkowalleck
jkowalleck merged commit 48b06b5 into master Feb 2, 2022
@jkowalleck
jkowalleck deleted the component_author-issue246 branch February 2, 2022 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderpatros coderpatros Awaiting requested review from coderpatros
@stevespringett stevespringett Awaiting requested review from stevespringett
3 more reviewers
@sonatype-lift sonatype-lift[bot] sonatype-lift[bot] left review comments
@brent-spiner brent-spiner brent-spiner left review comments
@shiraze shiraze shiraze requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

chore CI Continuous Integration documentation enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

JSON output does not guarantee component.version value Include component.author node in generated SBOM

3 participants

@jkowalleck @shiraze @brent-spiner