Skip to content

Conversation

@vil02
Copy link
Member

@vil02 vil02 commented Jun 1, 2023

This PR adds a very simple CI for this repo (and fixes some bugs in the existing *.nim files).

Note that there are still some warnings in the existing files.

The future work is definitely to setup this project using nimble, but this also would require things like project structure change, which is way too far beyond the scope of this PR.

Regarding the shellscripts: #21

Copy link
Contributor

@ZoomRmc ZoomRmc left a comment

Choose a reason for hiding this comment

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

Looks good, besides the proposed change and this:
I propose replacing the Bash scripts in this PR with a nimscript task from #21.

@Panquesito7 Panquesito7 added the enhancement New feature or request label Jun 1, 2023
@dlesnoff
Copy link
Collaborator

dlesnoff commented Jun 2, 2023

Do we want to require passing tests on devel?

Sorry I meant checking against devel too.
I would like to know if some code works under devel but does not work with the stable version.

@dlesnoff
Copy link
Collaborator

dlesnoff commented Jun 2, 2023

I do not want to setup a Nimble/Atlas project file. I have had many problems switching between developer and user versions of the repositories. I am confused by the recent changes brought to Nimble that made it quite unstable. ) The imposed Nimble directory structure will probably conflict with the directory structure of the Algorithms repository.

Nimble is not really necessary. What advantages does it really bring ?

  • Tasks in Nimble can be written in a classical .nims file.
  • Dependencies handling ? We do not want dependencies.

ZoomRmc
ZoomRmc previously approved these changes Jun 2, 2023
@vil02 vil02 requested a review from dlesnoff June 2, 2023 23:08
@vil02 vil02 requested a review from Panquesito7 June 3, 2023 07:13
@vil02 vil02 requested a review from dlesnoff June 3, 2023 12:39
@vil02 vil02 requested a review from dlesnoff June 3, 2023 18:53
dlesnoff
dlesnoff previously approved these changes Jun 3, 2023
Copy link
Collaborator

@dlesnoff dlesnoff left a comment

Choose a reason for hiding this comment

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

Improves greatly the repository's state and workflow. I need @Panquesito7 's approval to merge this.

@vil02 vil02 requested a review from Panquesito7 June 4, 2023 14:40
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.

Awesome. Thanks! 🚀

@dlesnoff dlesnoff merged commit 0f30aa9 into TheAlgorithms:main Jun 4, 2023
@vil02 vil02 deleted the add_nim_test branch June 4, 2023 16:52
@ZoomRmc
Copy link
Contributor

ZoomRmc commented Jun 4, 2023

Nice work everybody!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants