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

fix: .uccignore file is ignoring files specified #382

Closed
wants to merge 1 commit into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Oct 27, 2021

Closes #362

@ghost ghost mentioned this pull request Oct 27, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2021

Codecov Report

Merging #382 (a9f71ef) into main (5031177) will increase coverage by 0.57%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #382      +/-   ##
==========================================
+ Coverage   79.48%   80.06%   +0.57%     
==========================================
  Files          28       28              
  Lines        1867     1871       +4     
==========================================
+ Hits         1484     1498      +14     
+ Misses        383      373      -10     
Impacted Files Coverage Δ
splunk_add_on_ucc_framework/__init__.py 79.79% <88.88%> (+2.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5031177...a9f71ef. Read the comment docs.

@ghost ghost force-pushed the fix/uccignore branch from 2038aa9 to 9851cc1 Compare October 27, 2021 15:10
@ghost ghost marked this pull request as ready for review October 27, 2021 19:25
@ghost ghost enabled auto-merge (rebase) October 27, 2021 20:42
@ghost ghost force-pushed the fix/uccignore branch from 9851cc1 to a9f71ef Compare October 27, 2021 20:44
@github-actions
Copy link

Expected release notes (by @arys-splunk)

fixes:
.uccignore file is ignoring files specified (a9f71ef)

others (will not be included in Semantic-Release notes):
clean up pyproject.toml file (5031177)
add CHANGELOG generation on release (703775e)
pre-commit autoupdate (41da465)
remove syslog mention (de8e662)
bump cycjimmy/semantic-release-action from 2.5.4 to 2.6.0 (8c0e937)
initial docs (f931f90)
bump snyk/release-notes-preview from 1.6.1 to 1.6.2 (0bbe0c9)
bump docker/login-action from 1.9.0 to 1.10.0 (3575848)
bump codecov/codecov-action from 1 to 2.1.0 (c5f66ec)
add Github Actions to dependabot (75a33a3)
delete .jscpd file as not used (e6f30a4)
bump coverage from 6.0.1 to 6.0.2 (84cc6a1)
pyupgrade --py37-plus (82bacd5)
bump jsonschema from 4.0.1 to 4.1.0 (3f19ffb)
remove unnecessary comments (e3b1177)
bump coverage from 6.0 to 6.0.1 (3176ac1)
bump jinja2 from 3.0.1 to 3.0.2 (1cfbf76)
remove unnecessary dependabot configuration (466a497)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

@ghost ghost disabled auto-merge October 27, 2021 21:08
@harshpatel-crest
Copy link
Contributor

@arys-splunk I believe this was intended behaviour. Here is the PR: https://github.com/splunk/addonfactory-ucc-generator/pull/36/files. If I remember correctly the discussion I had, the purpose was to not ignore files that are provided in the package folder and only ignore those which are generated by UCC.

I will leave the decision of merging this to @rfaircloth-splunk

Copy link
Contributor

@ryanfaircloth ryanfaircloth left a comment

Choose a reason for hiding this comment

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

More internal discuessions, which is a reminder of the consequences of having undocumented temporary features. The original purpose of this file was to prevent the ucc-gen from using current templates for generated files while we were consolidating code and doing other cleanup activities. The ask here is actually a missing feature that closely resembles this one. Sorry @arys-splunk I lead you astray on this one.

  • close the current PR you have add a new PR with similar code as a new function, with the ".uccignorepackage" as the source file instead. Document both where the the current feature is "Trust the developer don't replace template files" and the new feature is "don't include in output"

Copy link
Contributor

@ryanfaircloth ryanfaircloth left a comment

Choose a reason for hiding this comment

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

More internal discuessions, which is a reminder of the consequences of having undocumented temporary features. The original purpose of this file was to prevent the ucc-gen from using current templates for generated files while we were consolidating code and doing other cleanup activities. The ask here is actually a missing feature that closely resembles this one. Sorry @arys-splunk I lead you astray on this one.

  • close the current PR you have add a new PR with similar code as a new function, with the ".uccignorepackage" as the source file instead. Document both where the the current feature is "Trust the developer don't replace template files" and the new feature is "don't include in output"

@ghost
Copy link
Author

ghost commented Oct 29, 2021

After clarifying one more time with Ryan, we need to have 2 files:

  1. .uccignore - if you add a file there - it means that ucc-gen should not rewrite this file with it's own content, example - import_declare_test.py with changes that you do not want to lose.
  2. .uccpackageignore - tell ucc-gen not to specific file / folder to the "output" directory, example - eventgen.conf.

@ghost ghost closed this Oct 29, 2021
@ghost ghost deleted the fix/uccignore branch October 29, 2021 21:43
@github-actions github-actions bot locked and limited conversation to collaborators Oct 29, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: add a way to delete files in the output folder
4 participants