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

Updates for documentation and some cleanups #371

Merged
merged 17 commits into from
Nov 6, 2024

Conversation

Lorak-mmk
Copy link

@Lorak-mmk Lorak-mmk commented Aug 16, 2024

This PR has some changes that can all be categorized as cleanups and various improvements:

  • Removes files that are relevant for upstream but not for us.
  • Updates README / README-dev / CONTRIBUTING because they were old and innacurate
  • Adds a PR template, copied from Rust Driver repo
  • Move examples to examples dir
  • Removes ez_setup
  • fixes one unit test, which allows to simplify unit test command by removing filter

For complete list and more details see commit messages.

@Lorak-mmk Lorak-mmk force-pushed the various-improvements branch 3 times, most recently from 827e1ef to a44ecaf Compare August 16, 2024 10:37
@Lorak-mmk Lorak-mmk marked this pull request as ready for review August 16, 2024 16:00
@Lorak-mmk Lorak-mmk changed the title [WIP] Updates for documentation and some cleanups Updates for documentation and some cleanups Aug 16, 2024
README-dev.rst Outdated Show resolved Hide resolved
README-dev.rst Outdated Show resolved Hide resolved
README-dev.rst Outdated Show resolved Hide resolved
Copy link

@fruch fruch left a comment

Choose a reason for hiding this comment

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

LGTM

We have just 3 scripts (1 soon to be gone because it is unnecessary),
so there is no need to have 2 folders for them.
Some of them are only used by upstream, some are may have been used by us
in the past.
Whatever the reason, we don't need them anymore, so this commit removes them.
This will introduce conflicts when merging upstream but:
- Those conflicts will be easy to solve - just pick our version
- When we move to more granular way of pulling changes we will get rid of
  redundant commits from history. We could even automatically drop commits
  that only touch those files.
No need for them to pollute main directory.
Forum is now the preferred venue for asking questions, this commit
updates our README to reflect that.
It is talking about Datastax CLA, which is not required to contribute to
our fork. Afaik we don't have any CLA required for external contributors
to our drivers.
@Lorak-mmk
Copy link
Author

Rebased on master.

Updated section about releasing, testing and documentation to match
current tooling and procedures.
Removed some sections that are no longer relevant.
It was copied from Rust Driver repo, with "CONTRIBUTING.md" changed to
"CONTRIBUTING.rst".
This is deprecated and no longer required.
This test verifies that the run method of the loop was called during setUp,
but another test (`class TestTwistedTimer`) was creating and starting the
loop before - and this loop was never stopped.

This commit adds cleanup procedure so that the test works even if loop
was started before.
We don't need them, and its better to remove them rather than
complicate test commands.
test_connection_initialization was fixed in previous commit
test_cloud was removed in previous commit
It's most likely no longer necessary
One of the badges was not working. That's a good opportunity to update them.
Now badges reflect status of master - not latest release - because that
better reflects the state of repository. I also added a badge for
integration tests.
@Lorak-mmk
Copy link
Author

Addressed @dkropachev comments.

@Lorak-mmk
Copy link
Author

I'll merge this. @dkropachev could you rebase #364 on master after the merge?

@Lorak-mmk Lorak-mmk merged commit 18f5f01 into scylladb:master Nov 6, 2024
20 checks passed
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