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

feat: leetcode zigzag solution #897

Merged

Conversation

straight-into-the-wall
Copy link
Contributor

@straight-into-the-wall straight-into-the-wall commented Oct 22, 2021

Description of Change

A solution for the ZigZag conversion problem.

References

Hacktoberfest issue

ZigZag Conversion problem on leetcode

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Relevant documentation/comments is changed or added
  • PR title follows semantic commit guidelines
  • Search previous suggestions before making a new one, as yours may be a duplicate.
  • I acknowledge that all my contributions will be made under the project's license.

Notes: Solution to the 6-ZigZag conversion problem was missing. Here is (decent) one.

ZigZagConversion-SubmissionDetail-LeetCode.pdf

@straight-into-the-wall straight-into-the-wall marked this pull request as ready for review October 22, 2021 14:29
@straight-into-the-wall
Copy link
Contributor Author

Hey @Panquesito7 @kvedala,

On my fork's Github Actions I got this error from Awesome CI Workflow Code formatter; even if I separated the two files into two different commits.

2 files are not in one and only one directory:
leetcode/src/1.c
leetcode/src/6.c

Error: Process completed with exit code 2.

Any idea how I could fix it?

Thanx

awesome_error

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Good work! 😄👍

leetcode/src/6.c Outdated Show resolved Hide resolved
leetcode/src/6.c Outdated Show resolved Hide resolved
leetcode/src/6.c Outdated Show resolved Hide resolved
leetcode/src/6.c Outdated Show resolved Hide resolved
@Panquesito7 Panquesito7 added Autochecks are failing Failing CI Auto-checks Changes requested enhancement New feature or request labels Oct 22, 2021
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Please fix clang-tidy warnings. Let us know if you need any help here or in our Discord server. 😃

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Still failing here.

@straight-into-the-wall
Copy link
Contributor Author

I've done clang-tidy --fix --quiet -p build leetcode/src/6.c -- on my side without any warning. The Awesome CI error message is quite cryptic 1 files are not in one and only one directory. I've checked and didn't find any other file named 6.c in the whole repo. ... waiting for help on #898 and Discord server.

@straight-into-the-wall
Copy link
Contributor Author

solved by PR #900 ?

@Panquesito7
Copy link
Member

Still failing. 👀

@straight-into-the-wall
Copy link
Contributor Author

straight-into-the-wall commented Oct 31, 2021

Somehow this PR was again tested with the buggy version of awesome_workflow.yml:

image

You can see in the details of the failing check.

image

and then in failing workflow file:

image

Master branch now seems to have the right .github/workflows/awesome_workflow.yml

image

... Maybe relaunch the tests ?

@Panquesito7 Panquesito7 removed the Autochecks are failing Failing CI Auto-checks label Oct 31, 2021
leetcode/src/6.c Outdated Show resolved Hide resolved
leetcode/src/6.c Outdated Show resolved Hide resolved
Co-authored-by: David Leal <[email protected]>
Co-authored-by: David Leal <[email protected]>
@Panquesito7
Copy link
Member

I've seen the tests are failing in other PRs with the same issue you had in this PR. Would you like to investigate? Thanks. 🙂

@straight-into-the-wall
Copy link
Contributor Author

I've seen the tests are failing in other PRs with the same issue you had in this PR. Would you like to investigate? Thanks. slightly_smiling_face

I checked some of the waiting PR but any of them has the same issue that this one. Could you link some references?

@Panquesito7
Copy link
Member

Panquesito7 commented Nov 8, 2021

I've seen the tests are failing in other PRs with the same issue you had in this PR. Would you like to investigate? Thanks. 🙂

I checked some of the waiting PR but any of them has the same issue that this one. Could you link some references?

#914

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

A question: there's no main function? 🤔

@straight-into-the-wall
Copy link
Contributor Author

straight-into-the-wall commented Nov 21, 2021

A question: there's no main function?

@Panquesito7

added main function.

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Almost there! 😄

leetcode/src/6.c Outdated Show resolved Hide resolved
leetcode/src/6.c Show resolved Hide resolved
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Thanks for your contribution! 😄👍

@Panquesito7 Panquesito7 added approved Approved; waiting for merge and removed Changes requested labels Nov 21, 2021
@straight-into-the-wall
Copy link
Contributor Author

@ayaankhan98 @mishraabhinn @Panquesito7 ... ping? 😉

@Panquesito7 Panquesito7 merged commit 37534f4 into TheAlgorithms:master Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved; waiting for merge enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants