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

(#293) Solved factor config_open_default_target out of wrapper.h #368

Merged
merged 11 commits into from
Sep 17, 2023

Conversation

rmknan
Copy link
Contributor

@rmknan rmknan commented Sep 16, 2023

The issue has been solved and the test run successfully. Made changes tostumpless.ymland deleted private/config/wrapper.h. New header file open_default_target.h created.

@rmknan
Copy link
Contributor Author

rmknan commented Sep 16, 2023

#293

@rmknan
Copy link
Contributor Author

rmknan commented Sep 16, 2023

I am on a different branch but it seems yesterday's commit have been re-committed. Is that fine?

@codecov
Copy link

codecov bot commented Sep 16, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (9fad9a2) 91.55% compared to head (c986a92) 91.55%.

Additional details and impacted files
@@           Coverage Diff           @@
##           latest     #368   +/-   ##
=======================================
  Coverage   91.55%   91.55%           
=======================================
  Files          43       43           
  Lines        3658     3658           
  Branches      474      474           
=======================================
  Hits         3349     3349           
  Misses        216      216           
  Partials       93       93           
Files Changed Coverage Δ
src/target.c 90.99% <ø> (ø)

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

Copy link
Owner

@goatshriek goatshriek left a comment

Choose a reason for hiding this comment

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

Thank you for putting this together! There are a few minor changes to make, listed below.

Also, be sure to merge in the latest branch to yours to remove the seemingly duplicate changes from the string formatting change.

include/private/config/wrapper/open_default_target.h Outdated Show resolved Hide resolved
include/private/config/wrapper/open_default_target.h Outdated Show resolved Hide resolved
src/target.c Outdated Show resolved Hide resolved
@goatshriek goatshriek added the refactor changes that require refactoring of existing code label Sep 16, 2023
@goatshriek goatshriek self-assigned this Sep 16, 2023
@rmknan
Copy link
Contributor Author

rmknan commented Sep 16, 2023

Also, be sure to merge in the latest branch to yours to remove the seemingly duplicate changes from the string formatting change.

Can you elaborate? For this I issue I created a new branch and after the changes I did git push -u origin <branch>. Are you asking me to do git push -u origin latest? or somthing else?

@goatshriek
Copy link
Owner

You need to perform a pull before you do another push. Exactly how you do the pull depends on how you have your local repository set up, but it's likely something like: git pull upstream latest. If you don't have a remote set up named upstream for this repository, you can do so with this command: git remote add upstream [email protected]:goatshriek/stumpless.git. If you run git log and see the commit regarding param to string format, you'll know that you have succeeded.

@goatshriek goatshriek merged commit 7a8c928 into goatshriek:latest Sep 17, 2023
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor changes that require refactoring of existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants