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

Don't console.log in Production #45

Closed
liammulh opened this issue Nov 19, 2021 · 1 comment · Fixed by #193
Closed

Don't console.log in Production #45

liammulh opened this issue Nov 19, 2021 · 1 comment · Fixed by #193
Labels
future Not being actively worked on, but might be worked on in the future good first issue Good for newcomers maintenance Functionality-orthogonal refactoring or code cleanup that needs doing

Comments

@liammulh
Copy link
Member

It's generally considered bad practice to use console.log in production. When we deploy, we need to make sure we don't have any console.log's.

It might be that npm run build strips out console.log's. We can address this when we're close to deploy.

@gwhitney gwhitney added the maintenance Functionality-orthogonal refactoring or code cleanup that needs doing label Nov 21, 2021
@gwhitney
Copy link
Collaborator

I recommend this issue remain until, and be closed when, there are no console.log commands anywhere in the code base that could be hit in production operation. (I am not actually clear on how far or near we are to that state.)

@gwhitney gwhitney added the good first issue Good for newcomers label Dec 9, 2021
gwhitney added a commit to gwhitney/frontscope that referenced this issue Feb 15, 2022
  To allow params to be of any type (rather than just `string | number | boolean`
  and to allow params to correspond to correctly-typed top-level properties,
  this commit creates a common base class Paramable for Sequences and
  Visualizers that handles communication of params with the UI.

  Basically, a class just derives from Paramable, defines a `params` property
  with objects describing the top-level properties that should be params,
  and defines a checkParameters() function that determines if a collection
  of candidate values (as `value` properties in the `params` object) are
  acceptable. If so, they are copied automatically into the implementing
  object's top-level properties. If the implementation changes any of these
  properties and wants the change to be reflected in the UI, it must
  call `this.refreshProperties()` manually.

  Also, extends SeqVizParamModal.vue to present reasonable UI interfaces
  for a much larger variety of param types, as well as adds features such as
  hidable params, indentation, and so on.

  Resolves numberscope#55.
  Resolves numberscope#56.
  Resolves numberscope#60.
  Resolves numberscope#62.
  Resolves numberscope#82.
  (Although the graphic presentation for collapsible subgroups could be
  more polished.)

  Also takes care of numberscope#45 in all sequences and visualizers (there are still
  a few gratuitous console.logs in components, though).
  Lays groundwork for numberscope#68, in that the channel for communication between
  the UI and the sequence/visualizer is now very clear and as close to
  direct as possible (I don't think we want it any more direct; if it were,
  then in the midst of typing the visualizer could be left in an invalid
  state.)
gwhitney added a commit to gwhitney/frontscope that referenced this issue Feb 15, 2022
  To allow params to be of any type (rather than just `string | number | boolean`
  and to allow params to correspond to correctly-typed top-level properties,
  this commit creates a common base class Paramable for Sequences and
  Visualizers that handles communication of params with the UI.

  Basically, a class just derives from Paramable, defines a `params` property
  with objects describing the top-level properties that should be params,
  and defines a checkParameters() function that determines if a collection
  of candidate values (as `value` properties in the `params` object) are
  acceptable. If so, they are copied automatically into the implementing
  object's top-level properties. If the implementation changes any of these
  properties and wants the change to be reflected in the UI, it must
  call `this.refreshProperties()` manually.

  Also, extends SeqVizParamModal.vue to present reasonable UI interfaces
  for a much larger variety of param types, as well as adds features such as
  hidable params, indentation, and so on.

  Resolves numberscope#55.
  Resolves numberscope#56.
  Resolves numberscope#60.
  Resolves numberscope#62.
  Resolves numberscope#82.
  (Although the graphic presentation for collapsible subgroups could be
  more polished.)

  Also takes care of numberscope#45 in all sequences and visualizers (there are still
  a few gratuitous console.logs in components, though).
  Lays groundwork for numberscope#68, in that the channel for communication between
  the UI and the sequence/visualizer is now very clear and as close to
  direct as possible (I don't think we want it any more direct; if it were,
  then in the midst of typing the visualizer could be left in an invalid
  state.)
gwhitney added a commit to gwhitney/frontscope that referenced this issue Feb 15, 2022
  To allow params to be of any type (rather than just `string | number | boolean`
  and to allow params to correspond to correctly-typed top-level properties,
  this commit creates a common base class Paramable for Sequences and
  Visualizers that handles communication of params with the UI.

  Basically, a class just derives from Paramable, defines a `params` property
  with objects describing the top-level properties that should be params,
  and defines a checkParameters() function that determines if a collection
  of candidate values (as `value` properties in the `params` object) are
  acceptable. If so, they are copied automatically into the implementing
  object's top-level properties. If the implementation changes any of these
  properties and wants the change to be reflected in the UI, it must
  call `this.refreshProperties()` manually.

  Also, extends SeqVizParamModal.vue to present reasonable UI interfaces
  for a much larger variety of param types, as well as adds features such as
  hidable params, indentation, and so on.

  Resolves numberscope#55.
  Resolves numberscope#56.
  Resolves numberscope#60.
  Resolves numberscope#62.
  Resolves numberscope#82.
  (Although the graphic presentation for collapsible subgroups could be
  more polished.)

  Also takes care of numberscope#45 in all sequences and visualizers (there are still
  a few gratuitous console.logs in components, though).
  Lays groundwork for numberscope#68, in that the channel for communication between
  the UI and the sequence/visualizer is now very clear and as close to
  direct as possible (I don't think we want it any more direct; if it were,
  then in the midst of typing the visualizer could be left in an invalid
  state.)
gwhitney added a commit to gwhitney/frontscope that referenced this issue Feb 15, 2022
  To allow params to be of any type (rather than just `string | number | boolean`
  and to allow params to correspond to correctly-typed top-level properties,
  this commit creates a common base class Paramable for Sequences and
  Visualizers that handles communication of params with the UI.

  Basically, a class just derives from Paramable, defines a `params` property
  with objects describing the top-level properties that should be params,
  and defines a checkParameters() function that determines if a collection
  of candidate values (as `value` properties in the `params` object) are
  acceptable. If so, they are copied automatically into the implementing
  object's top-level properties. If the implementation changes any of these
  properties and wants the change to be reflected in the UI, it must
  call `this.refreshProperties()` manually.

  Also, extends SeqVizParamModal.vue to present reasonable UI interfaces
  for a much larger variety of param types, as well as adds features such as
  hidable params, indentation, and so on.

  Resolves numberscope#55.
  Resolves numberscope#56.
  Resolves numberscope#60.
  Resolves numberscope#62.
  Resolves numberscope#82.
  (Although the graphic presentation for collapsible subgroups could be
  more polished.)

  Also takes care of numberscope#45 in all sequences and visualizers (there are still
  a few gratuitous console.logs in components, though).
  Lays groundwork for numberscope#68, in that the channel for communication between
  the UI and the sequence/visualizer is now very clear and as close to
  direct as possible (I don't think we want it any more direct; if it were,
  then in the midst of typing the visualizer could be left in an invalid
  state.)
liammulh pushed a commit that referenced this issue Mar 28, 2022
* refactor: first rough cut and direct parameter access (avoid typing step)

* refactor: Overhaul param system

  To allow params to be of any type (rather than just `string | number | boolean`
  and to allow params to correspond to correctly-typed top-level properties,
  this commit creates a common base class Paramable for Sequences and
  Visualizers that handles communication of params with the UI.

  Basically, a class just derives from Paramable, defines a `params` property
  with objects describing the top-level properties that should be params,
  and defines a checkParameters() function that determines if a collection
  of candidate values (as `value` properties in the `params` object) are
  acceptable. If so, they are copied automatically into the implementing
  object's top-level properties. If the implementation changes any of these
  properties and wants the change to be reflected in the UI, it must
  call `this.refreshProperties()` manually.

  Also, extends SeqVizParamModal.vue to present reasonable UI interfaces
  for a much larger variety of param types, as well as adds features such as
  hidable params, indentation, and so on.

  Resolves #55.
  Resolves #56.
  Resolves #60.
  Resolves #62.
  Resolves #82.
  (Although the graphic presentation for collapsible subgroups could be
  more polished.)

  Also takes care of #45 in all sequences and visualizers (there are still
  a few gratuitous console.logs in components, though).
  Lays groundwork for #68, in that the channel for communication between
  the UI and the sequence/visualizer is now very clear and as close to
  direct as possible (I don't think we want it any more direct; if it were,
  then in the midst of typing the visualizer could be left in an invalid
  state.)

* fix: changes as requested in review

* docs: Clarify the meanings of ParamInterface properties

* feat: Add 'integer' forceType for Paramable

  This special type for a parameter stores its result in a `number` but
  enforces in the UI that the value be an integer.
  This commit also adds this forceType to all Sequence and Visualizer
  parameters for which it is appropriate. That change noticeably reduces the
  amount of validation that needs to occur in the TypeScript code.

* feat(params): When an input box "fixes" integer input, post a type reminder

* refactor(param): Remove "indent" prop in favor of style for dependent params
@liammulh liammulh added the future Not being actively worked on, but might be worked on in the future label Jun 7, 2022
gwhitney pushed a commit that referenced this issue Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future Not being actively worked on, but might be worked on in the future good first issue Good for newcomers maintenance Functionality-orthogonal refactoring or code cleanup that needs doing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants