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

Fix sentence and remove existing action comments #2067

Merged
merged 3 commits into from
Apr 13, 2023

Conversation

arjundashrath
Copy link
Contributor

@arjundashrath arjundashrath commented Apr 9, 2023

closes #2056
closes #2064

@varunsh-coder I rebased it. This PR has both the bug fixes

@arjundashrath arjundashrath changed the base branch from int to main April 10, 2023 16:57
@arjundashrath arjundashrath changed the base branch from main to int April 10, 2023 16:58
@arjundashrath arjundashrath changed the title Fix sentence Fix sentence and remove existing action comments Apr 10, 2023
@@ -76,6 +76,13 @@ func PinAction(action, inputYaml string) (string, bool) {
pinnedAction := fmt.Sprintf("%s@%s # %s", leftOfAt[0], commitSHA, tagOrBranch)
updated = !strings.EqualFold(action, pinnedAction)
inputYaml = strings.ReplaceAll(inputYaml, action, pinnedAction)
stringParts := strings.SplitN(inputYaml, pinnedAction, 2)
Copy link
Member

Choose a reason for hiding this comment

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

@Devils-Knight can you please review this code? I think you had worked on this code recently, so just review this logic once. Also confirm if this change is on the latest from main.

@Devils-Knight
Copy link
Contributor

@varunsh-coder the above logic looks fine to me and also the changes are on latest with reference to main.


steps:
- name: Close Issue
uses: peter-evans/close-issue@v1 #Mock comment to remove
Copy link
Member

Choose a reason for hiding this comment

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

@arjundashrath can you please update this test workflow to have two jobs and use the same Action with comment in both jobs? Also, have another Action in one of the jobs with a comment. I want to check if it will fix this for multiple instances and more complex workflows.

@arjundashrath
Copy link
Contributor Author

@varunsh-coder added a more complex test case

@varunsh-coder varunsh-coder merged commit 830739d into step-security:int Apr 13, 2023
varunsh-coder added a commit that referenced this pull request Apr 13, 2023
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.

3 participants