-
-
Notifications
You must be signed in to change notification settings - Fork 7k
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: added LeetCode problem 1,2 #2450
base: master
Are you sure you want to change the base?
Conversation
Created new directory for leetcode solutions
- name: Write LeetCode DIRECTORY.md | ||
run: | | ||
python3 scripts/leetcode_directory_md.py 2>&1 | tee leetcode/DIRECTORY.md | ||
git pull || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, this is not needed if we use fetch-depth
in actions/checkout
.
CC: @tjgurwara99.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is not needed.
Co-authored-by: David Leal <halfpacho@gmail.com>
leetcode/README.md
Outdated
|
||
3. Do not provide a `main` function. Use the required standalone functions instead. | ||
4. Doxygen documentation isn't used in LeetCode solutions. Simple/small documentation or comments should be fine. | ||
5. Don't include libraries/headers such as `iostream`. Your file should be the solution to the problem only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be needed to be added in the files to prevent CI failures, unless we ignore the files in the CI, which is not 100% recommended, though.
What do you suggest we do here? I suggest using libraries/headers in the files to prevent CI failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking whether this directory could be excluded from the CI, as in the case of the C repo leetcode directory. Else we also need to add a driver code for each problem along with some test cases. What are your thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need a main
function or self-tests for the CI to pass, we just need to make the function work as expected with its necessary libraries/headers.
AFAIK, the C repository doesn't ignore the leetcode
directory in the CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Panquesito7
Updated the cpp files and the README file for these changes.
if: steps.commit-push.outputs.changes_detected == 'true' | ||
run: | | ||
gh pr create --base ${GITHUB_REF##*/} --head leetcode-directory-${{ github.sha }} --title 'docs: updating `leetcode/DIRECTORY.md`' --body 'Updated LeetCode directory (see the diff. for changes).' | ||
gh pr merge --admin --merge --subject 'docs: updating `leetcode/DIRECTORY.md' --delete-branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm personally against merging PR's through a workflow but I leave this decision to your @Panquesito7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to remove that part of the code so that we can check the PR to see if it is 100% fine.
Reviewing wouldn't take long as I'm always aware of incoming PRs in this repository. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the yml file @Panquesito7 @tjgurwara99
* Updated the leetcode Readme.md file
# when doing a git push | ||
name: leetcode_directory_writer | ||
on: | ||
push: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this for every push? 🤔 I think it suffices to have this for a pull_request instead... What do you think @Panquesito7?
Hi @Panquesito7, @tjgurwara99 /home/runner/work/C-Plus-Plus/C-Plus-Plus/leetcode/src/2.cpp:29:9: warning: assigning newly created 'gsl::owner<>' to non-owner 'ListNode *' [cppcoreguidelines-owning-memory] I guess it is because of the use of raw pointers instead of smart pointers. Is there a way to suppress this? (This may require changing the definition of the ListNode struct, which is already defined by the platform) struct ListNode { |
Does this directory also fall under doxygen style guidelines? |
I believe so. Not required, but if added, it's better, IMO. |
It'd be better to use smart pointers, though. |
Ah alright! |
This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Please ping one of the maintainers once you commit the changes requested or make improvements on the code. If this is not the case and you need some help, feel free to ask for help in our Gitter channel or our Discord server. Thank you for your contributions! |
Created new directory for leetcode solutions, updated DIRECTORY.md and README.md
Description of Change
Checklist
Notes: