Skip to content

Fix --insert-license-after-regex to work beyond first line #103

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.

Sign up for GitHub

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

Jump to bottom
Merged
Lucas-C merged 1 commit into from
Jun 5, 2025
Merged

Fix --insert-license-after-regex to work beyond first line #103

Lucas-C merged 1 commit into from
Jun 5, 2025

Conversation

@ashb
Copy link
Contributor

@ashb ashb commented Apr 25, 2025 •
edited
Loading

There was a logic bug where index wasn't incremenetd, so we
would search through the file for the regex, but index was still 0
so the index always got put after line 0.

Without this fix, in the test PHP file we ended up with this:

<!-- Hi -->
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements.  See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership.  The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License.  You may obtain a copy of the License at
//
//   http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied.  See the License for the
// specific language governing permissions and limitations
// under the License.

<html>
<?php
print "Hello";

@ashb
Copy link
Contributor Author

ashb commented May 8, 2025

K, this has a glitch where if the regex is not found it inserts it before the last line.

@amoghrajesh
Copy link

amoghrajesh commented May 26, 2025

@Lucas-C can we please get a review on this PR when you have some time?

@Lucas-C
Copy link
Owner

Lucas-C commented May 26, 2025

Thank you for this fix @ashb!

Could you please rebase this PR?
It is currently failing du to a Pylint check that I just disabled on the master branch (too-many-positional-arguments):
24c29be

@ashb
Fix --insert-license-after-regex to work beyond first line
494ddb4 
There was a logic bug where `index` wasn't incremenetd, so we
would search through the file for the regex, but index was still 0
so the index always got put after line 0.
@ashb
Copy link
Contributor Author

ashb commented Jun 5, 2025

@Lucas-C Rebased!

@Lucas-C Lucas-C merged commit fd3fbe8 into Lucas-C:master Jun 5, 2025
8 checks passed
@Lucas-C
Copy link
Owner

Lucas-C commented Jun 5, 2025

Merged! Thank you @ashb 👍

@ashb ashb deleted the fix-insert-after-regex branch June 5, 2025 15:17
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 6, 2025
@potiuk
Workaround the need to use latest version on Lucas pre-commit's
a18d2f1 
We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 6, 2025
@potiuk
Workaround the need to use latest version on Lucas pre-commit's
d1e9aac 
We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
@potiuk potiuk mentioned this pull request Jul 6, 2025
Merged
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 6, 2025
@potiuk
Workaround the need to use latest version on Lucas pre-commit's
b1070d6 
We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 6, 2025
@potiuk
Workaround the need to use latest version on Lucas pre-commit's
fb81fee 
We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
potiuk added a commit to apache/airflow that referenced this pull request Jul 6, 2025
@potiuk
Workaround the need to use latest version on Lucas pre-commit's (#52931)
535d71d 
We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 6, 2025
@potiuk
Workaround the need to use latest version on Lucas pre-commit's (apac…
ba2c3b0 
…he#52931)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.

(cherry picked from commit 535d71d)
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 6, 2025
@potiuk
[v3-0-test] Workaround the need to use latest version on Lucas pre-co…
97d3cd1 
…mmit's (apache#52931)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
(cherry picked from commit 535d71d)

Co-authored-by: Jarek Potiuk <[email protected]>
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 6, 2025
@potiuk
[v3-0-test] Workaround the need to use latest version on Lucas pre-co…
b3eef83 
…mmit's (apache#52931)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
(cherry picked from commit 535d71d)

Co-authored-by: Jarek Potiuk <[email protected]>
potiuk added a commit to apache/airflow that referenced this pull request Jul 6, 2025
@potiuk
[v3-0-test] Workaround the need to use latest version on Lucas pre-co…
a48189d 
…mmit's (#52931) (#52942)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
(cherry picked from commit 535d71d)
kaxil pushed a commit to apache/airflow that referenced this pull request Jul 7, 2025
@potiuk @kaxil
[v3-0-test] Workaround the need to use latest version on Lucas pre-co…
25a7579 
…mmit's (#52931) (#52942)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
(cherry picked from commit 535d71d)
kaxil pushed a commit to apache/airflow that referenced this pull request Jul 7, 2025
@potiuk @kaxil
[v3-0-test] Workaround the need to use latest version on Lucas pre-co…
05f11ff 
…mmit's (#52931) (#52942)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
(cherry picked from commit 535d71d)
kaxil pushed a commit to apache/airflow that referenced this pull request Jul 9, 2025
@potiuk @kaxil
[v3-0-test] Workaround the need to use latest version on Lucas pre-co…
3323f43 
…mmit's (#52931) (#52942)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
(cherry picked from commit 535d71d)
HsiuChuanHsu pushed a commit to HsiuChuanHsu/airflow that referenced this pull request Jul 10, 2025
@potiuk @HsiuChuanHsu
Workaround the need to use latest version on Lucas pre-commit's (apac…
94030c3 
…he#52931)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
kaxil pushed a commit to apache/airflow that referenced this pull request Jul 11, 2025
@potiuk @kaxil
[v3-0-test] Workaround the need to use latest version on Lucas pre-co…
1edbd2d 
…mmit's (#52931) (#52942)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
(cherry picked from commit 535d71d)
kaxil pushed a commit to apache/airflow that referenced this pull request Jul 11, 2025
@potiuk @kaxil
[v3-0-test] Workaround the need to use latest version on Lucas pre-co…
51645de 
…mmit's (#52931) (#52942)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
(cherry picked from commit 535d71d)
stephen-bracken pushed a commit to stephen-bracken/airflow that referenced this pull request Jul 15, 2025
@potiuk
Workaround the need to use latest version on Lucas pre-commit's (apac…
dc6e7a8 
…he#52931)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
kosteev pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this pull request Sep 25, 2025
@potiuk
[v3-0-test] Workaround the need to use latest version on Lucas pre-co…
63c085d 
…mmit's (#52931) (#52942)

We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.
(cherry picked from commit 535d71d91f36d3d9859d32e4695f0e81a2696a4b)

GitOrigin-RevId: 51645de62637eee2b3130c3ba64323b05c4792e2
kosteev pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this pull request Oct 23, 2025
@potiuk
Workaround the need to use latest version on Lucas pre-commit's (#52931)
f12920e 
We are still waiting for release of the Lucas's pre-commit after
the Lucas-C/pre-commit-hooks#103 has
been merged - and for now we need to use `bleeding-edge` for
the repo.

Also upgrades zizmor and solves using "env." in shell commands
that might lead to security issues.

GitOrigin-RevId: 535d71d91f36d3d9859d32e4695f0e81a2696a4b
@potiuk potiuk mentioned this pull request Dec 13, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lucas-C Lucas-C Lucas-C approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ashb @amoghrajesh @Lucas-C