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

Update doc for changes in wrappers #167

Merged
merged 11 commits into from
Nov 18, 2024
Merged

Update doc for changes in wrappers #167

merged 11 commits into from
Nov 18, 2024

Conversation

clostao
Copy link
Contributor

@clostao clostao commented Nov 12, 2024

User description

Changes in #161 were not reflected in the doc. This PR updates it


PR Type

Documentation


Description

  • Updated the README.md files for both auto-dag-data and auto-drive packages to reflect recent changes in function names and usage.
  • Introduced new examples demonstrating the use of MemoryBlockstore and updated function names for creating IPLD DAGs.
  • Added detailed sections on handling observables, uploading files from different interfaces, and downloading files using the auto-drive API.
  • Improved documentation to enhance usability and understanding of the new features and changes.

Changes walkthrough 📝

Relevant files
Documentation
README.md
Update function names and examples in README                         

packages/auto-dag-data/README.md

  • Updated function names for creating IPLD DAGs.
  • Added examples for new function usage.
  • Introduced MemoryBlockstore in examples.
  • +26/-16 
    README.md
    Revise examples and add new sections in README                     

    packages/auto-drive/README.md

  • Revised examples for file upload and download.
  • Added sections for handling observables and custom interfaces.
  • Updated API initialization examples.
  • +171/-20

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    github-actions bot commented Nov 12, 2024

    PR Reviewer Guide 🔍

    (Review updated until commit ead7a3f)

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    161 - Fully compliant

    Fully compliant requirements:

    • Update blake3 lib for @webuf/blake3 for enabling usage in browser
    • Add tools for uploading from browser (supporting File interface)
    • Add encrypted folder logic to this package (on encrypted folders upload a zip is generated and is uploaded as any encrypted file)
    • Add observability through rxjs package in downloads
    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Code Consistency
    The new code introduces processFileToIPLDFormat and processFolderToIPLDFormat replacing createFileIPLDDag and createFolderIPLDDag. Ensure that these changes are consistent across all modules and that they do not break existing functionalities.

    Interface Clarity
    The introduction of GenericFile interface for file abstraction needs clear documentation to ensure it's correctly implemented by users, especially regarding the read() method and its usage.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Correct the function usage to match the imported function to prevent runtime errors

    Ensure that the uploadFile function is correctly imported and used since the code
    snippet shows uploadFileFromFilepath being imported but uploadFile being used. This
    might lead to a runtime error if uploadFile is not defined elsewhere.

    packages/auto-drive/README.md [41-50]

     import { uploadFileFromFilepath } from '@autonomys/auto-drive'
     ...
    -const uploadObservable = uploadFile(api, filePath, options)
    +const uploadObservable = uploadFileFromFilepath(api, filePath, options)
    Suggestion importance[1-10]: 9

    Why: The suggestion correctly identifies a mismatch between the imported function and the one used in the code, which could lead to runtime errors. Fixing this is crucial for the functionality of the code.

    9
    Ensure that the buffer is initialized with actual data to avoid errors

    Replace the Buffer.from(...) with actual data or ensure that the placeholder is
    replaced with valid data to prevent runtime errors.

    packages/auto-drive/README.md [81]

    -const buffer = Buffer.from(...);
    +const buffer = Buffer.from('your actual data here');
    Suggestion importance[1-10]: 7

    Why: This suggestion is valid as it ensures that the buffer initialization is done with actual data, preventing potential runtime errors from a placeholder value.

    7
    Enhancement
    Implement error handling in the download function to manage potential exceptions effectively

    Add error handling for the downloadFile function to manage exceptions that may occur
    during the file download process, enhancing the robustness of the code.

    packages/auto-drive/README.md [182-186]

     const downloadFile = async (cid) => {
    -  const stream = await downloadObject(api, { cid })
    -  console.log('File downloaded successfully:', stream)
    +  try {
    +    const stream = await downloadObject(api, { cid })
    +    console.log('File downloaded successfully:', stream)
    +  } catch (error) {
    +    console.error('Error downloading file:', error)
    +  }
     }
    Suggestion importance[1-10]: 8

    Why: Adding error handling to the download function is a significant improvement for robustness, especially in network operations where errors are common.

    8
    Best practice
    Validate the progress value to ensure it falls within the expected range before updating the state

    Consider checking the progress value to ensure it is within a valid range (0-100)
    before setting it to state in the React example, to prevent UI errors or
    inconsistencies.

    packages/auto-drive/README.md [157]

    -setProgress(status.progress)
    +if (status.progress >= 0 && status.progress <= 100) {
    +  setProgress(status.progress)
    +}
    Suggestion importance[1-10]: 6

    Why: Validating the progress value is a good practice to maintain UI consistency and prevent errors, though it's not as critical as fixing functional bugs or errors.

    6

    Base automatically changed from fix/file-not-supported to main November 13, 2024 09:20
    @clostao clostao dismissed marc-aurele-besner’s stale review November 13, 2024 09:20

    The base branch was changed.

    @clostao
    Copy link
    Contributor Author

    clostao commented Nov 13, 2024

    I've updated the @autonomys/auto-dag-data doc too that I noticed it was outdated

    @marc-aurele-besner
    Copy link
    Collaborator

    What is the blocker to merge this PR?

    @clostao
    Copy link
    Contributor Author

    clostao commented Nov 16, 2024

    What is the blocker to merge this PR?

    No one now

    @clostao clostao closed this Nov 16, 2024
    @clostao clostao reopened this Nov 16, 2024
    Copy link

    Persistent review updated to latest commit ead7a3f

    Copy link

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @clostao
    Copy link
    Contributor Author

    clostao commented Nov 16, 2024

    The skip on workflow led to not be mergeable I tried to close & reopen but didn't work either. I'd need another reviewal

    @clostao clostao merged commit b0f5db0 into main Nov 18, 2024
    1 check passed
    @clostao clostao deleted the update/auto-drive-doc branch November 18, 2024 14:34
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants