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(#162): moved repo type #164

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat(#162): moved repo type #164

wants to merge 2 commits into from

Conversation

l3r8yJ
Copy link
Collaborator

@l3r8yJ l3r8yJ commented Sep 24, 2024

the POC


PR-Codex overview

This PR introduces a restructuring of the codebase, primarily focusing on the organization of modules and the addition of new dependencies, including the kill-port package. It also refactors the repo module into repo_ops, enhancing the functionality related to repository operations.

Detailed summary

  • Added kill-port dependency in package.json and package-lock.json.
  • Updated Cargo.toml to include a new types module.
  • Renamed repo module to repo_ops in server/src/objects/mod.rs.
  • Introduced RepoOperations trait in repo_ops.rs for better repo management.
  • Added Repo struct in types/src/types/repo.rs.
  • Updated User struct in types/src/types/user.rs to include repos.
  • Added GitHub struct in types/src/types/github.rs for GitHub representation.
  • Included MIT license headers in new files.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@l3r8yJ l3r8yJ requested a review from h1alexbel September 24, 2024 15:20
@l3r8yJ l3r8yJ self-assigned this Sep 24, 2024
# Conflicts:
#	server/Cargo.toml
Copy link

codecov bot commented Sep 24, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.15%. Comparing base (febccfb) to head (d4f71f7).

Files with missing lines Patch % Lines
server/src/objects/repo_ops.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #164   +/-   ##
=======================================
  Coverage   83.15%   83.15%           
=======================================
  Files          16       16           
  Lines         285      285           
=======================================
  Hits          237      237           
  Misses         48       48           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@h1alexbel h1alexbel left a comment

Choose a reason for hiding this comment

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

@l3r8yJ Are you plan to use types only for fsl compiler output, or potentially something else? Take a look at my comments below

@@ -0,0 +1,5 @@
{
Copy link
Owner

Choose a reason for hiding this comment

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

@l3r8yJ this file should be not pushed to the branch. Let's add it to the .gitignore

# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
# SOFTWARE.
[package]
name = "types"
Copy link
Owner

Choose a reason for hiding this comment

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

@l3r8yJ it's better to name package fakehub-types, instead of types. Thus we will be able to publish this package. WDYT?

# SOFTWARE.
[package]
name = "types"
version = "0.1.0"
Copy link
Owner

Choose a reason for hiding this comment

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

@l3r8yJ version should be freezed (0.0.0), I believe

*/
/// The types.
pub mod types;
// @todo #162:25min Remove dumb documentation.
Copy link
Owner

@h1alexbel h1alexbel Sep 24, 2024

Choose a reason for hiding this comment

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

@l3r8yJ I think these docs is set by default for lib.rs and main.rs files, correct me if I'm wrong

The types.
*/
/// The types.
pub mod types;
Copy link
Owner

Choose a reason for hiding this comment

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

@l3r8yJ I do understand the purpose of this types library, but other potential users/developers don't. Let's create README.md inside types that will explain what is that, and how it works. Similar way we did with server, and cli

@@ -22,3 +22,5 @@ github-mirror/.gitignore
github-mirror/git_push.sh
github-mirror/README.md
github-mirror/.openapi-generator-ignore
github-mirror/**/src
Copy link
Owner

Choose a reason for hiding this comment

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

@l3r8yJ we can't ignore github-mirror/src because, when we release fakehub, we package and push github-mirror to crates too

@@ -0,0 +1,14 @@
[package]
Copy link
Owner

Choose a reason for hiding this comment

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

@l3r8yJ this should file will be autogenerated, we don't need to keep it in the repo

@@ -38,7 +38,8 @@ path = "src/lib.rs"
workspace = true

[dependencies]
openapi = { path = "../github-mirror", version = "1.1.4" }
openapi = { path = "../github-mirror" }
Copy link
Owner

Choose a reason for hiding this comment

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

@l3r8yJ version attribute is required for release runtime and sed command in .rultor.yml file


/// GitHub.
#[derive(Clone)]
pub struct GitHub {
Copy link
Owner

Choose a reason for hiding this comment

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

@l3r8yJ we have GitHub in server/objects/github.rs. We should somehow resolve ambiguity with naming here

@l3r8yJ
Copy link
Collaborator Author

l3r8yJ commented Sep 24, 2024

@h1alexbel this PR still in WIP status, miss clicked on review button

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