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

check_license: accept Triple Slash Directive #1098

Merged
merged 1 commit into from
Jul 20, 2020

Conversation

blankoworld
Copy link
Contributor

run-tests.sh checks that Licenses are OK for some files.
This scripts fails when JS files use a Triple Slash Directive.
This commit change this.

Co-Authored-by: Olivier DOSSMANN [email protected]

Why are you opening this PR?

Because Cypress tests needs to be done on RERO ILS and I need to extend developer environment to do so.

How to test?

poetry run run-tests shouldn't give any error on Cypress JS files (that contains Triple Slash Directive)

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

@blankoworld blankoworld requested review from AoNoOokami and rerowep and removed request for AoNoOokami July 17, 2020 14:18
@blankoworld blankoworld marked this pull request as ready for review July 17, 2020 14:19
Copy link

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

Commit message approved.

run-tests.sh checks that Licenses are OK for some files.
This scripts fails when JS files use a Triple Slash Directive.
This commit change this.

* Improves check_license method to include Triple-Slash directives for
.js files (Cf.
https://www.typescriptlang.org/docs/handbook/triple-slash-directives.html)
* Avoids checking screenshots directory in Cypress
* Adds a triple slash directive on 2 JS files (from Cypress).

Co-Authored-by: Olivier DOSSMANN <[email protected]>
@blankoworld blankoworld force-pushed the doo-triple-dash-directives branch from 4584532 to cf69684 Compare July 20, 2020 08:26
@blankoworld blankoworld merged commit 82bc44b into rero:dev Jul 20, 2020
@blankoworld blankoworld deleted the doo-triple-dash-directives branch July 20, 2020 08:57
Copy link
Contributor

@lauren-d lauren-d left a comment

Choose a reason for hiding this comment

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

not tested. Just do code review

Comment on lines +453 to +456
is_js_file = file.name.split('.')[-1] == 'js'
if is_js_file and re.search(triple_slash, line):
return True
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

cosmetic: can be replaced by:

if file.name.split('.')[-1] == 'js' and re.search(triple_slash, line):
   return True

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.

4 participants