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: validate if config is a DAG #32

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

legosandorigami
Copy link
Contributor

@scriptnull

This PR addresses issue #15 by implementing and providing the following four test cases:

  1. Valid DAG
    1_valid_dag

  2. Valid DAG
    2_valid_dag

  3. Invalid DAG
    3_invalid_dag

  4. Invalid DAG
    4_invalid_dag

To maintain the encapsulation of the Connector struct, I kept the fields unexported. However, I introduced the following two methods to the Connector interface:

From() string
To() string

I have successfully run the tests locally and confirmed that everything is working as expected. Please feel free to review and suggest any improvements or changes. I apologize in advance for any oversights.

@scriptnull
Copy link
Member

Hi, Thanks for the pull request! I will get back to you once I find some free time to review it!

@legosandorigami
Copy link
Contributor Author

legosandorigami commented Sep 9, 2024

@scriptnull,

I wanted to acknowledge and apologize for an oversight in the cycle detection algorithm I contributed. I initially used code from ChatGPT without fully verifying it. While the rest of the work in the PR was original, I realize this was a lapse in my process.

I have corrected the algorithm and tested it to ensure its accuracy. My data structures and algorithms skills were a bit rusty, which contributed to this oversight. I am committed to improving and ensure that this does not happen again.

Thank you for your understanding and for reviewing my PR.

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.

2 participants