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

Added some documentations to AsyncLock file #2627

Merged
merged 2 commits into from
Oct 5, 2024
Merged

Conversation

herlandro
Copy link
Contributor

No description provided.

@ankushkushwaha
Copy link

@herlandro thanks for this addition. There are some failed workflow, I dont know what is the cause of failing these workflows. Could you please try to fix these?

@herlandro
Copy link
Contributor Author

@ankushkushwaha I saw that most of the PRs made after July 17th presented the same workflow test error. In all the logs I analyzed, the error is the same:
/opt/build/repo: error: package at '/opt/build/repo' is using Swift tools version 5.5.0 but the installed version is 5.4.0
The problem is that the Swift version in the build environment is 5.4, but the project requires Swift 5.5.0:

  • There are some places to check: .yml files, scritps (Netlify or Travis), package.swift.
  • Also in the logs they mention that there was an attempt to restore an old cache, it may be necessary to clear the cache to avoid conflicts between different versions of Swift.

The first file that we can attack is: .github/workflows/tests.yml

@freak4pc
Copy link
Member

The main issue is that the CI is still using an older version of Xcode, I'll try to get to tackling it soon and bump to Xcode 16.

@herlandro
Copy link
Contributor Author

@freak4pc Did you see about the Xcode update to version 16?

Comment on lines 49 to 34
run: |
sudo apt-get update
sudo apt-get install -y curl libssl-dev
git clone https://github.com/kylef/swiftenv.git ~/.swiftenv
echo 'export SWIFTENV_ROOT="$HOME/.swiftenv"' >> ~/.bashrc
echo 'export PATH="$SWIFTENV_ROOT/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(swiftenv init -)"' >> ~/.bashrc
export SWIFTENV_ROOT="$HOME/.swiftenv"
export PATH="$SWIFTENV_ROOT/bin:$PATH"
eval "$(swiftenv init -)"
swiftenv install 5.5
swiftenv global 5.5
swift --version

Copy link
Member

Choose a reason for hiding this comment

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

I'm unsure how this change relates to documentation updates. Would you mind removing it from this PR and have a separate one with an explanation on why it's needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. Done!

@freak4pc
Copy link
Member

freak4pc commented Oct 3, 2024

I've rebased your branch -
The previous CI failure was on docs generation which isn't really an issue because it's not a required check. Yet, I fixed it.
Please remove the updates to the GH workflows and leave only the doc updates.

Thanks 🙏

Copy link
Member

@freak4pc freak4pc left a comment

Choose a reason for hiding this comment

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

Ok, so - tl;dr
I personally don't think it's usually worth the effort to document so deeply the internal layers of RxSwift, since it's not public interface that consumers are using -

But since you've already done this, it definitely doesn't hurt. Thank you for the work!

@freak4pc freak4pc merged commit 43423f8 into ReactiveX:main Oct 5, 2024
16 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.

4 participants