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.

Sign up for GitHub

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

Jump to bottom

Implement the truly "Header only" version #224

Closed
wants to merge 3 commits into from
Closed

Implement the truly "Header only" version #224

wants to merge 3 commits into from

Conversation

amerinor01
Copy link
Contributor

  • Remove CXXGraphConfig.h.in dependency.
  • Set a flag to Enable/Disable Gtests (off by default)
  • Remove the zlib dependency unless you are using GTests or Benchmark, and set the installation path of CXXGraph into /usr/include/CXXGraph

This commit should close the Issue #209.
Waiting @ZigRazor to review the incoming changes.

@github-actions github-actions bot added core something about core repo something about repo labels Oct 5, 2022
@ghost
Copy link

ghost commented Oct 5, 2022

👇 Click on the image for a new way to code review

  • Make big changes easier — review code in small groups of related files

  • Know where to start — see the whole change at a glance

  • Take a code tour — explore the change with an interactive tour

  • Make comments and review — all fully sync’ed with github

    Try it now!

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

@ZigRazor ZigRazor self-requested a review October 5, 2022 11:15
ZigRazor
ZigRazor requested changes Oct 5, 2022
View reviewed changes
Copy link
Owner

@ZigRazor ZigRazor left a comment

Choose a reason for hiding this comment

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

please fix the comment

include/Graph/Graph.hpp
@@ -2186,7 +2190,7 @@ namespace CXXGRAPH
}
return result;
}

#ifdef WITH_COMPRESSION
Copy link
Owner

Choose a reason for hiding this comment

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

readFromFile and writeToFile functions, should have 2 different implementation for compression enabled, and disabled. We can't disable the readFromFile and writeToFile function only because no compression is set.

include/CXXGraph.hpp
@@ -1,7 +1,11 @@
#ifndef __CXXGRAPH_H__
#define __CXXGRAPH_H__

#include "CXXGraphConfig.h"
// the configured options and settings for CXXGraph
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe this file should not exist anymore, if generated when the cmake is called.

@ZigRazor ZigRazor linked an issue Oct 5, 2022 that may be closed by this pull request
Truly "header only" version #209
Closed
@ZigRazor
Copy link
Owner

@amerinor01 are you working on it?

@nrkramer
Copy link
Collaborator

nrkramer commented May 6, 2023

Tangentially, this PR would close #275

@nrkramer
Copy link
Collaborator

nrkramer commented May 6, 2023 •
edited
Loading

@ZigRazor @amerinor01 Given that this PR has been open for a year, would anyone mind if I created a new PR to address this issue?

@ZigRazor
Copy link
Owner

ZigRazor commented May 6, 2023

@nrkramer You can create a new Pr, I will close this!

@nrkramer nrkramer mentioned this pull request May 6, 2023
Merged
@nrkramer
Copy link
Collaborator

nrkramer commented May 6, 2023 •
edited
Loading

Opened a new PR here: #276. This can be closed.

@ZigRazor
Copy link
Owner

ZigRazor commented May 7, 2023

Closed for new PR #276

@ZigRazor ZigRazor closed this May 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZigRazor ZigRazor ZigRazor requested changes

Assignees
No one assigned
Labels
core something about core repo something about repo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Truly "header only" version
3 participants
@amerinor01 @ZigRazor @nrkramer