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

credits #285

Closed
pixelzoom opened this issue Dec 19, 2018 · 18 comments
Closed

credits #285

pixelzoom opened this issue Dec 19, 2018 · 18 comments

Comments

@pixelzoom
Copy link
Contributor

Finalize the credits in wave-interference-main.js.

This was referenced Dec 19, 2018
arouinfar added a commit that referenced this issue Jan 2, 2019
@arouinfar
Copy link
Contributor

The credits are currently up-to-date, but @KatieWoe will need to finalize the QA credits when this sim is in the last round of rc testing.

@DianaTavares you are currently credited as Diana López. Is this how you would like to be credited, or would you prefer Diana López Tavares?

@arouinfar arouinfar assigned DianaTavares and unassigned arouinfar Jan 2, 2019
@DianaTavares
Copy link

I prefer Diana López Tavares, thanks!

@pixelzoom
Copy link
Contributor Author

Hmm... Diana's credits seem to be a little inconsistent across sims. In Equality Explorer, she is "Diana Berenice López Tavares". In Area Model suite and Interaction Dashboard, she is "Diana Lopez Tavares" (López incorrect).

@DianaTavares
Copy link

I suggest always use Diana López Tavares

arouinfar added a commit to phetsims/area-model-decimals that referenced this issue Jan 2, 2019
arouinfar added a commit to phetsims/area-model-decimals that referenced this issue Jan 2, 2019
arouinfar added a commit to phetsims/area-model-introduction that referenced this issue Jan 2, 2019
arouinfar added a commit to phetsims/area-model-introduction that referenced this issue Jan 2, 2019
arouinfar added a commit to phetsims/area-model-algebra that referenced this issue Jan 2, 2019
arouinfar added a commit to phetsims/area-model-algebra that referenced this issue Jan 2, 2019
arouinfar added a commit to phetsims/area-model-multiplication that referenced this issue Jan 2, 2019
arouinfar added a commit to phetsims/area-model-multiplication that referenced this issue Jan 2, 2019
arouinfar added a commit to phetsims/equality-explorer that referenced this issue Jan 2, 2019
arouinfar added a commit to phetsims/equality-explorer that referenced this issue Jan 2, 2019
@arouinfar
Copy link
Contributor

arouinfar commented Jan 2, 2019

Thanks @DianaTavares @pixelzoom. I've updated the sim credits to always use Diana López Tavares.

Assigning to @KatieWoe to finalize the QA credits when this sim is in rc testing.

@pixelzoom
Copy link
Contributor Author

@arouinfar Other developers may disagree... But I typically avoid putting non-ASCII characters (like the 'ó' in "López") is source code files. Instead I use the Unicode escapes for non-ASCII characters (like "Diana L\u00f3pez Tavares" in EqualityExplorerConstants). Definitely get another developer opinion, but consider sticking to ASCII.

@arouinfar
Copy link
Contributor

Thanks @pixelzoom. I was unaware of such a convention. @samreid (I presume) used ó in wave-interference-main.js, which is the pattern I followed.

I have seen non-ASCII characters in sim credits before, including hookes-law-main.js, which has a ç.

If this is a convention, then I will make the dozen-or-so commits to change the ó it to \u00f3, but perhaps this should be an item on the code review checklist.

@samreid
Copy link
Member

samreid commented Jan 3, 2019

Thanks @jonathanolson. @arouinfar can you convert credits to use ascii and escapes?

UPDATE: Thanks @pixelzoom!

@samreid samreid removed their assignment Jan 3, 2019
arouinfar added a commit to phetsims/hookes-law that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/hookes-law that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/hookes-law that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/area-model-decimals that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/area-model-decimals that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/area-model-introduction that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/area-model-introduction that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/area-model-algebra that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/area-model-algebra that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/area-model-multiplication that referenced this issue Jan 3, 2019
arouinfar added a commit to phetsims/area-model-multiplication that referenced this issue Jan 3, 2019
@arouinfar
Copy link
Contributor

I have replaced ó with \u00f3 and ç with \u00e7 in sim credits.

@pixelzoom @samreid @jonathanolson I would recommend adding something to the code review checklist to check for non-ASCII characters in .js files. @DianaTavares will be credited in several more sims, so we are likely to run into ó showing up in sim credits again.

@samreid
Copy link
Member

samreid commented Jan 4, 2019

Here's a lint rule that would prevent that:

// Copyright 2018, University of Colorado Boulder

/**
 * Lint detector for invalid text based on regex search.  Checks the entire file and does not correctly report line number.
 *
 * @author Sam Reid (PhET Interactive Simulations)
 */
module.exports = function( context ) {
  'use strict';

  var badRegexes = [
    /[^\u0000-\u007F]+/
  ];

  return {
    Program: function( node ) {
      var sourceCode = context.getSourceCode();
      var text = sourceCode.text;
      badRegexes.forEach( function( badRegex ) {
        if ( badRegex.test( text ) ) {
          context.report( {
            node: node,
            message: 'File contains bad regex: \'' + badRegex + '\''
          } );
        }
      } );
    }
  };
};

module.exports.schema = [
  // JSON Schema for rule options goes here
];

@samreid
Copy link
Member

samreid commented Jan 4, 2019

@pixelzoom I opened phetsims/chipper#732 and @arouinfar used escape codes in Wave Interference credits. Anything else you are aware of?

If not, we'll leave assigned to @KatieWoe according to this comment from above:

Assigning to @KatieWoe to finalize the QA credits when this sim is in rc testing.

@pixelzoom
Copy link
Contributor Author

Nothing else that I'm aware of.

@pixelzoom pixelzoom removed their assignment Jan 4, 2019
@KatieWoe
Copy link
Contributor

Kathryn Woessner, Liam Mulhal, Jacob Romero, Kelly Wurtz, Luara Rea, and Megan Lai

samreid added a commit that referenced this issue Jan 24, 2019
@samreid
Copy link
Member

samreid commented Jan 24, 2019

Credits updated in the preceding commit. @KatieWoe can you please double check? If all is well, please close.

@KatieWoe
Copy link
Contributor

Looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants