Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

file-header fix for single-line comments #2320

Merged
merged 1 commit into from
Mar 9, 2017
Merged

Conversation

battmanz
Copy link
Contributor

@battmanz battmanz commented Mar 8, 2017

The file-header rule was not correctly parsing single-line comments. Therefore the regex was not working. That is now fixed.

PR checklist

Overview of change:

I fixed the commentRegexp so that it will correctly parse single-line comments. It was an easy fix. I simply removed a ?.

Is there anything you'd like reviewers to focus on?

Please make sure I made the correct decision with the tests inside the empty directory.

CHANGELOG.md entry:

[bugfix] file-header-rule now handles single-line comments correctly

The file-header rule was not correctly parsing single-line comments.
Therefore the regex was not working. That is now fixed.
@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint, @battmanz! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@@ -4,9 +4,9 @@
*/

export class A {
public x = 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this is supposed to be a .js file test, I removed the visibility modifiers.

@@ -1 +1,2 @@
// empty.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously this file had just a single-line comment. That case is now covered by both the good-single-line and bad-single-line tests. I have modified this file to truly be empty.

@adidahiya
Copy link
Contributor

thanks @battmanz!

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

Successfully merging this pull request may close these issues.

4 participants